Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(files): Fix type issues of mixing string and number #41952

Closed
wants to merge 1 commit into from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Dec 1, 2023

Summary

Fix some type issues in the files list where numbers were mixed with strings for fileids and thus the strict compare will fail (===).

Checklist

@susnux susnux force-pushed the fix/files-typescript-issues branch 3 times, most recently from 5858325 to 46a8eff Compare December 1, 2023 16:57
@susnux susnux requested review from artonge, skjnldsv and ShGKme December 1, 2023 16:57
@@ -194,8 +194,8 @@ export default Vue.extend({
},

computed: {
currentView(): View {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the typing?

Copy link
Contributor Author

@susnux susnux Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it was wrong, the correct type would be View | null as this is the type of Navigation.active .
So the correct typing is inferred.

@@ -486,7 +492,9 @@ export default defineComponent({
if (window?.OCA?.Files?.Sidebar?.setActiveTab) {
window.OCA.Files.Sidebar.setActiveTab('sharing')
}
sidebarAction.exec(this.currentFolder, this.currentView, this.currentFolder.path)
if (this.currentFolder !== undefined) {
Copy link
Member

@skjnldsv skjnldsv Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like there is a lot of unrelated changes?
How would this.currentFolder be undefined?
The FilesList gets rendered only if we have files, which we have only if this.currentFolder is defined, no? 🤔

EDIT: saw the above changes! 👍

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/files-typescript-issues branch from 46a8eff to 050ce4d Compare December 13, 2023 15:52
@susnux susnux requested a review from skjnldsv December 13, 2023 15:54
@susnux susnux added this to the Nextcloud 29 milestone Jan 20, 2024
This was referenced Mar 12, 2024
@skjnldsv
Copy link
Member

Too much conflicts, closing 🙈

@skjnldsv skjnldsv closed this Mar 15, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Mar 15, 2024
@skjnldsv skjnldsv deleted the fix/files-typescript-issues branch March 15, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants