Skip to content

Comments

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

Merged
pmuetschard merged 7 commits intogoogle:masterfrom
stellama0208:ui_checkin
Jun 10, 2019
Merged

Make two UI modifications in Welcoming window and Help-About dialog.#2752
pmuetschard merged 7 commits intogoogle:masterfrom
stellama0208:ui_checkin

Conversation

@stellama0208
Copy link
Contributor

@stellama0208 stellama0208 commented May 14, 2019

  • 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

stellama0208 commented May 21, 2019

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

@stellama0208
Copy link
Contributor Author

stellama0208 commented May 21, 2019

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
private Label recentIcon;
private Link recentLink;
private Label helpIcon;
private Link helpLink;
Copy link
Member

@pmuetschard pmuetschard Jun 4, 2019

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

@stellama0208 stellama0208 Jun 4, 2019

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)

pmuetschard
pmuetschard reviewed Jun 6, 2019
View reviewed changes
@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