Skip to content

Fix: Navigating using left/right arrows in the column layout no longer crashes the app #10421

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Nov 14, 2022
Merged

Fix: Navigating using left/right arrows in the column layout no longer crashes the app #10421

merged 22 commits into from
Nov 14, 2022

Conversation

QuaintMako
Copy link
Contributor

Resolved / Related Issues
Items resolved / related issues by this PR.

What has been done

  • Programmatically handling the change of focus between the columns.
  • Minor refactoring.

Validation
How did you test these changes?

  • Built and ran the app

Screenshots

CrashTabFixDraft

@QuaintMako QuaintMako added help wanted Extra attention is needed needs - code review labels Nov 7, 2022
@QuaintMako QuaintMako changed the title Fix: Navigating using arrows in ColumnView no longer crashes the app Fix: Navigating using left/right arrows in ColumnView no longer crashes Files Nov 7, 2022
@QuaintMako
Copy link
Contributor Author

The work for this PR is almost done, but I cannot find a cleaner way to refresh the selection when backing up to the previous folder between line.222 and line.226 in ColumnViewBrowser.xaml.cs. If anyone has an idea, I'll gladly take it.

@QuaintMako QuaintMako marked this pull request as ready for review November 8, 2022 17:28
@QuaintMako
Copy link
Contributor Author

The work for this PR is almost done, but I cannot find a cleaner way to refresh the selection when backing up to the previous folder between line.222 and line.226 in ColumnViewBrowser.xaml.cs. If anyone has an idea, I'll gladly take it.

Cannot find a cleaner way to solve this problem, so I'm opening it for review.

@QuaintMako QuaintMako removed the help wanted Extra attention is needed label Nov 8, 2022
ferrariofilippo
ferrariofilippo suggested changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@ferrariofilippo ferrariofilippo left a comment

Choose a reason for hiding this comment

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

There are some problems with the code style and lines 402 -> 410 (src.Files.App/Views/LayoutModes/ColumnViewBase.xaml.cs), otherwise it is fine.

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Nov 10, 2022 •
edited
Loading

I may have found a bug but I'm not sure if it's related to this PR and the way we focus a blade.
Sometimes, while navigating between columns, it happens that the focus bounces between the last two blades.
I post here a video to show what happens.

1.mp4

Everything else works as intended

@QuaintMako
Copy link
Contributor Author

I may have found a bug but I'm not sure if it's related to this PR and the way we focus a blade. Sometimes, while navigating between columns, it happens that the focus bounces between the last two blades. I post here a video to show what happens.

I'm going to investigate this. It is probably linked to this PR, since beforehand we would not even move between the blades.

@QuaintMako
Copy link
Contributor Author

QuaintMako commented Nov 11, 2022 •
edited
Loading

After messing around a bit with the feature, I think it's not linked to the PR at all. Moving up and down with the arrows messes the display as well. I did not touch those. I propose to open another issue about this<
Thoughts @ferrariofilippo ?

EDIT: May be this one #9103

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Nov 12, 2022 •
edited
Loading

I propose to open another issue about this

I'll do so.
EDIT: I'm not sure this is related to #9103

ferrariofilippo
ferrariofilippo approved these changes Nov 12, 2022
View reviewed changes
Copy link
Contributor

@ferrariofilippo ferrariofilippo left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 requested a review from d2dyno1 November 13, 2022 16:04
@yaira2 yaira2 changed the title Fix: Navigating using left/right arrows in ColumnView no longer crashes Files Fix: Navigating using left/right arrows in the column layout no longer crashes the app Nov 14, 2022
yaira2
yaira2 approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 merged commit 821c4cb into files-community:main Nov 14, 2022
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Nov 14, 2022
@QuaintMako QuaintMako deleted the 10109_CrashArrowColumnView branch January 3, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaira2 yaira2 yaira2 approved these changes

@d2dyno1 d2dyno1 Awaiting requested review from d2dyno1

+1 more reviewer

@ferrariofilippo ferrariofilippo ferrariofilippo approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Using left/right arrow key in column layout crashes app
3 participants
@QuaintMako @ferrariofilippo @yaira2