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

added hover action to expand/collapse buttons and removed onclick on Watcher tab in Game/App lab #46887

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

fisher-alice
Copy link
Contributor

@fisher-alice fisher-alice commented Jun 16, 2022

Summary of changes:

-Added color change hover action to expand/collapse buttons on bottom of page (Debug console and Watcher area)
Although the Jira ticket specified a black hover color, I chose to change color to white since the code already included the color change to white on hover action for both buttons. Added a span around the the two icons allowed color of icons and pointer change to occur.
-Although not in Jira ticket, I also removed onclick on Watcher tab so that onclick for Watcher area active only on expand/collapse button.
Video of changes: https://youtu.be/MEGaCbvkpqs

jira ticket: https://codedotorg.atlassian.net/browse/STAR-2326?atlOrigin=eyJpIjoiMDQ1YWJjZWJmYTFlNDk1NDhjYjYyNjU2ZGQ4YTE0YTYiLCJwIjoiaiJ9

Follow-up work

Noticed that for Toolbox area at top of page, onclick/hover is active on Show Toolbox (when Toolbox is collapsed) but not on Toolbox (when Toolbox is expanded).
Originally, The Watchers tab had onclick whether it was expanded (displaying 'Watchers') or collapsed (displaying 'Show Watchers'). However, the hover action for pointer was not on the 'Show Watchers' as it is on the Show Toolbox.
Next task could be to add hover pointer for 'Show Watchers' label.

@fisher-alice fisher-alice requested review from a team and megcrenshaw June 17, 2022 16:45
@fisher-alice fisher-alice changed the title added hover action on span added hover action to expand/collapse buttons and removed onclick on Watcher tab in Game/App lab Jun 17, 2022
Copy link
Contributor

@megcrenshaw megcrenshaw left a comment

Choose a reason for hiding this comment

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

The video you linked is amazing––I'd love for you to teach me how to add those quotation bubbles to explain the change. The code looks good too, but I'm biased ;), so I'd wait for someone else to review it too. Well done!

@@ -631,7 +637,7 @@ const styles = {
fontSize: 18,
':hover': {
cursor: 'pointer',
color: 'white'
color: color.white
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting the hover color for the bottom buttons as white was the right call after looking at the code. Given that the Jira ticket specified a black hover color, put a brief note in the PR description explaining why you went with white in this case. That will help build understanding and shared context in case someone sees the change in the future and wants to adjust it to a different color.

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 the feedback! Just edited the comment and included rationale for why I went with white.

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

applause

Great work! Nice job on your first PR!

@fisher-alice fisher-alice merged commit 678de21 into staging Jun 17, 2022
@fisher-alice fisher-alice deleted the alice/add-hover-action branch June 17, 2022 22:57
@breville
Copy link
Member

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants