Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

viewer toolbar: align to right #901

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented Sep 17, 2021

Description

This pull request moves the alignment of the "hammer and screwdriver" toolbar to the right and the child "save" menu to the left so it does not overflow past the far right of the viewer.

Fixes #659

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
  • Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
  • Do the proposed changes follow the STScI Style Guides?
  • Are tests added/updated as required? If so, do they follow the STScI Style Guides?
  • Are docs added/updated as required? If so, do they follow the STScI Style Guides?
  • Did the CI pass? If not, are the failures related?
  • Is a change log needed? If yes, is it added to CHANGES.rst?
  • Is a milestone set? Milestone is only currently required for PRs related to Imviz MVP.
  • After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

* v-toolbar: apply inline css to align right (with 2px padding), removed right property which, as far as I can tell from testing and the vue API docs, does nothing.
* child save v-menu: align left so doesn't overflow into the next viewer (since the parent button is now on the far right)
@github-actions github-actions bot added the embed Regarding issues with front-end embedding label Sep 17, 2021
@codecov
Copy link

codecov bot commented Sep 17, 2021

Codecov Report

Merging #901 (c5c6888) into main (34df475) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #901   +/-   ##
=======================================
  Coverage   68.38%   68.38%           
=======================================
  Files          66       66           
  Lines        4799     4799           
=======================================
  Hits         3282     3282           
  Misses       1517     1517           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b28cfd...c5c6888. Read the comment docs.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Definitely feels more intuitive. Thanks!

p.s. Not sure what the "child save menu" is.

Before

Screenshot 2021-09-17 134041

After

Screenshot 2021-09-17 134430

@kecnry
Copy link
Member Author

kecnry commented Sep 17, 2021

p.s. Not sure what the "child save menu" is.

The menu with "save as png" and "save as svg" that pops up if you click on the save icon. It used to extend to the right of the save icon, but that would now overflow past the edge of the viewer (if there are multiple viewers like in Cubeviz). I changed that to always extend to the left so it doesn't overflow.

With that change (as is in the PR):
Screen Shot 2021-09-17 at 1 52 14 PM

Without that change:
Screen Shot 2021-09-17 at 1 53 16 PM

@pllim
Copy link
Contributor

pllim commented Sep 17, 2021

Ah! I obviously never try to click that button. 😆

Looks nice. Would approve again! -- ⭐⭐⭐⭐⭐

Copy link
Contributor

@javerbukh javerbukh left a comment

Choose a reason for hiding this comment

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

Great change, thank you!

@javerbukh javerbukh merged commit 32eb0c4 into spacetelescope:main Sep 17, 2021
@kecnry kecnry deleted the v-toolbar-right branch September 17, 2021 19:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
embed Regarding issues with front-end embedding Ready for final review UI/UX😍
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shift viewer toolbar under hammer and screwdriver to the right
3 participants