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.

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

Make two UI modifications in Welcoming window and Help-About dialog. #2752

Merged
merged 7 commits into from
Jun 10, 2019
Merged

Make two UI modifications in Welcoming window and Help-About dialog. #2752

merged 7 commits into from
Jun 10, 2019

Conversation

stellama0208
Copy link
Contributor

  • Replace the pop-up welcoming dialog with a in-panel welcoming page.
  • Remove loading messages.
  • Add click-to-copy shortcut icon in Help-About Dialog.
  • Change the layout of Help-About Dialog.

@stellama0208 stellama0208 requested a review from pmuetschard May 14, 2019 22:59
@pmuetschard
Copy link
Member

pmuetschard commented May 16, 2019 •
edited
Loading

I merged this in and tried it out. Some observations:

  1. The status message when opening gapid no longer shows. It just shows the logo for a few seconds and then the links appear.
  2. The Logo moves when the links appear.
  3. Please make the copy icon in the about dialog an actual button, so there is feedback when it is pressed. Also, please make it a 16x16 button, it seems too big right now.

stellama0208 added a commit to stellama0208/gapid that referenced this pull request May 21, 2019
@stellama0208
Further UI modifications based on comments in pull request google#2752.
5009659
stellama0208 added a commit to stellama0208/gapid that referenced this pull request May 21, 2019
@stellama0208
Make further UI modifications based on comments in pull request googl…
726d4dd 
…e#2752.
@stellama0208
Copy link
Contributor Author

Thanks for the advice! Have made another commit to solve the problems mentioned above.

@stellama0208
Copy link
Contributor Author

Btw, the above force-pushed history is just to amend the commit message.

@stellama0208
Make two UI modifications in Welcoming window and Help-About dialog.
4ff7944 
 - Replace the pop-up welcoming dialog with a in-panel welcoming page.
 - Remove loading messages.
 - Add click-to-copy shortcut icon in Help-About Dialog.
 - Change the layout of Help-About Dialog.
@stellama0208
Make further UI modifications based on comments in pull request googl…
b0c7c71 
…e#2752.
@stellama0208
crush the pngs
d553518
@stellama0208
Add back help option in the welcoming page.
c33a505
@pau-baiget
Copy link
Contributor

pau-baiget commented Jun 3, 2019 •
edited
Loading

I'd like this as part of the next sprint demo :) I'd be happy to approve this PR but i'd like @pmuetschard to give a final OK.

@pmuetschard
Copy link
Member

pmuetschard commented Jun 3, 2019 •
edited
Loading

Sorry, Stella and I looked at it last week and there were a couple of minor things that are still missing here that we talked about, but didn't update the PR.
Stella wrote them down, but here is what I remember:

  • slight misalignment of the hints
  • ctrl vs cmd for mac
  • bring back the help link
  • update the text for the open recent link

pmuetschard
pmuetschard reviewed Jun 4, 2019
View reviewed changes
Copy link
Member

@pmuetschard pmuetschard left a comment

Choose a reason for hiding this comment

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

Getting close!

gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/LoadingScreen.java Outdated
private Label recentIcon;
private Link recentLink;
private Label helpIcon;
private Link helpLink;
Copy link
Member

Choose a reason for hiding this comment

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

The labels and links can be final.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for all these constructive opinions! Done all the others except this one. Because I initialize my recentLink outside of the constructor. (Cause that part of initialization is huge). Can I keep it or should I still use some tricks to turn it into final? (Like returning recentLink as a variable in the other initializing part)

gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/views/AboutDialog.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/views/AboutDialog.java Outdated Show resolved Hide resolved
pmuetschard
pmuetschard reviewed Jun 6, 2019
View reviewed changes
gapic/src/main/com/google/gapid/views/AboutDialog.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/widgets/Widgets.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/views/AboutDialog.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/views/AboutDialog.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/LoadingScreen.java Outdated Show resolved Hide resolved
gapic/src/main/com/google/gapid/MainWindow.java Outdated Show resolved Hide resolved
@pmuetschard pmuetschard merged commit 67c8949 into google:master Jun 10, 2019
@stellama0208 stellama0208 deleted the ui_checkin branch July 26, 2019 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmuetschard pmuetschard pmuetschard approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stellama0208 @pmuetschard @pau-baiget @googlebot