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: Fixed crash in Dual Pane mode when switching away from Git folder #12599

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

gave92
Copy link
Member

@gave92 gave92 commented Jun 13, 2023

Resolved / Related Issues

There seem to be an issue with x:Bind so that you can't use a TwoWay x:Bind on an element that is inside an element with x:Load:

<StackPanel x:Load="{x:Bind ...}">
    <RadioButton IsChecked="{x:Bind ..., Mode=TwoWay}" />
</StackPanel>

Workaround is to use Binding.

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Open a non-git folder in left pane and set layout to Tiles
    2. Open a git folder in right pane and set layout to Details
    3. Switch focus between the two panes
    4. Verify that you can still switch between local and remote branches in the Git flyout

@gave92 gave92 changed the title Switch to binding Fix: Fixed crash in Dual Pane mode when getting out of Git folder Jun 13, 2023
Copy link
Contributor

@QuaintMako QuaintMako left a comment

Choose a reason for hiding this comment

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

LGTM and fixes the crash.

@gave92 gave92 marked this pull request as draft June 13, 2023 18:29
@gave92
Copy link
Member Author

gave92 commented Jun 13, 2023

Uops I've just tested again and it crashes with a different error. Re-checking.

@QuaintMako
Copy link
Contributor

Weird, I did not have any exception or crash while trying in the same set-up.

@gave92 gave92 marked this pull request as ready for review June 13, 2023 18:42
@gave92
Copy link
Member Author

gave92 commented Jun 13, 2023

Ok the second error I think it was caused by running "git status" in cmd while having that folder open in dual pane.

@yaira2 yaira2 changed the title Fix: Fixed crash in Dual Pane mode when getting out of Git folder Fix: Fixed crash in Dual Pane mode when switching away from Git folder Jun 13, 2023
@yaira2 yaira2 added the ready to merge Pull requests that are approved and ready to merge label Jun 13, 2023
@yaira2 yaira2 merged commit 34853e1 into files-community:main Jun 13, 2023
@gave92 gave92 deleted the issue_12589 branch June 13, 2023 19:36
@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Jun 18, 2023

There are two problems related with this PR:

  • You can't checkout branches (it will keep on checking out because SelectedIndex won't be reset to 0)
  • Navigating to a git repository, going back and then reopening the repo will cause Files to create +80 branches (The Locals radio button will be unchecked)

Do you have any idea on how to fix these two issues?

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Jun 18, 2023

Navigating to a git repository, going back and then reopening the repo will cause Files to create +80 branches (The Locals radio button will be unchecked)

We could solve this by changing the binding to OneWay and manually updating ShowLocals when the user clicks one of the two radios.
What do you think? @gave92 @yaira2 @QuaintMako

@yaira2
Copy link
Member

yaira2 commented Jun 18, 2023

Sounds good

@QuaintMako
Copy link
Contributor

It could work I believe

hishitetsu added a commit to hishitetsu/Files that referenced this pull request Jun 19, 2023
yaira2 pushed a commit that referenced this pull request Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Consistent crash in Dual Pane mode when getting out of Git folder
4 participants