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

Feature: Added support for local branch checkout #12316

Merged
merged 21 commits into from
May 11, 2023

Conversation

ferrariofilippo
Copy link
Contributor

@ferrariofilippo ferrariofilippo commented May 10, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Related Feature: Git integration #11014

Notes

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 folder with git repo with uncommitted changes
    2. Try switching branches from the status bar flyout
    3. Try all the available options

Screenshots (optional)
image
1

@ferrariofilippo ferrariofilippo marked this pull request as draft May 10, 2023 07:23
@ferrariofilippo ferrariofilippo marked this pull request as ready for review May 10, 2023 07:47
@0x5bfa
Copy link
Member

0x5bfa commented May 10, 2023

Shouldn't Branches, main, pr/12302 labels aligned in the same line?

@ferrariofilippo
Copy link
Contributor Author

Do you mean like this?

image

@0x5bfa
Copy link
Member

0x5bfa commented May 10, 2023

Yeah, but reduce left and right paddings? I guess have to set style of flyout padding so that you dont have to make paddings negative.

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented May 10, 2023

Yeah, but reduce left and right paddings? I guess have to set style of flyout padding so that you dont have to make paddings negative.

We can't set neither padding nor margin for the popup.
Using Margin = -8 this is what it looks like:
4

I may change that to Top/Bottom: 0 and Right/Left: -8
image

@0x5bfa
Copy link
Member

0x5bfa commented May 10, 2023

You can try this. But UI thing is not a priprity; we can do that after.

<Flyout.FlyoutPresenterStyle>
   <Style TargetType="FlyoutPresenter">
        <Setter Property="Padding" Value="8" />
        <Setter Property="CornerRadius" Value="{StaticResource OverlayCornerRadius}" />
    </Style>
</Flyout.FlyoutPresenterStyle>

@yaira2
Copy link
Member

yaira2 commented May 10, 2023

Shouldn't Branches, main, pr/12302 labels aligned in the same line?

The UI will be changed a bit in phase 3 so we can make these changes then.

@yaira2
Copy link
Member

yaira2 commented May 10, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaira2
Copy link
Member

yaira2 commented May 10, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaira2 yaira2 closed this May 10, 2023
@yaira2 yaira2 reopened this May 10, 2023
@yaira2 yaira2 changed the title Feature: Support local branch checkout Feature: Added suppotr for local branch checkout May 11, 2023
@ferrariofilippo ferrariofilippo changed the title Feature: Added suppotr for local branch checkout Feature: Added support for local branch checkout May 11, 2023
yaira2
yaira2 previously approved these changes May 11, 2023
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

hecksmosis
hecksmosis previously approved these changes May 11, 2023
Copy link
Contributor

@hecksmosis hecksmosis left a comment

Choose a reason for hiding this comment

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

I tested out of curiosity, and works ok for me!

@ferrariofilippo
Copy link
Contributor Author

Wait I just found a bug

@yaira2 yaira2 added the ready to merge Pull requests that are approved and ready to merge label May 11, 2023
@ferrariofilippo ferrariofilippo dismissed stale reviews from hecksmosis and yaira2 via 7d0db5b May 11, 2023 17:05
@ferrariofilippo
Copy link
Contributor Author

Can you confirm that everything works properly if you switch to a non git directory?

@yaira2
Copy link
Member

yaira2 commented May 11, 2023

Can you confirm that everything works properly if you switch to a non git directory?

It works after your latest fix.

@yaira2 yaira2 merged commit a8df728 into files-community:main May 11, 2023
@ferrariofilippo ferrariofilippo deleted the Git_Checkout branch May 11, 2023 18:29
@files-community files-community deleted a comment from samialgradee May 12, 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.

4 participants