-
Notifications
You must be signed in to change notification settings - Fork 33
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
repo picker: move 'add repo' functionality to a menu item or a separate button. #1468 #1486
repo picker: move 'add repo' functionality to a menu item or a separate button. #1468 #1486
Conversation
b6edf3a
to
91cb8db
Compare
567e3d4
to
c1a4053
Compare
Ready for review! |
88a6d26
to
f223605
Compare
Can post a screenshot? |
Optional<String> chosenRepo = dialog.showAndWait(); | ||
if (chosenRepo.isPresent() && !defaultRepo.equals(chosenRepo.get())) { | ||
onValueChangeCallback.accept(chosenRepo.get()); | ||
if (repoId != null && !defaultRepo.equals(repoId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realised the repo picker isn't a Dialog subclass, and so doesn't have access to showAndWait
.
Instead of using null, could we pass an optional instead? That's a bit of an abuse of optionals (their use case is as return values, not parameters), but since we're not using showAndWait
I guess it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was, but apparently I couldn't get it to spawn another dialog (the addRepository dialog one). So I had to make it not subclassing Dialog. Is it okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's fine.
f223605
to
e541c37
Compare
Done. Ready for another review! |
e541c37
to
4e83a48
Compare
4e83a48
to
aefbc5c
Compare
aefbc5c
to
be8da2c
Compare
No description provided.