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

1249 - Tapestry 3.0 - Use halo to indicate selected node rather than size change #1250

Merged
merged 7 commits into from
Nov 27, 2022
Merged

1249 - Tapestry 3.0 - Use halo to indicate selected node rather than size change #1250

merged 7 commits into from
Nov 27, 2022

Conversation

zifgu
Copy link
Contributor

@zifgu zifgu commented Aug 26, 2022 •
edited by Bitajkr
Loading

Mockups

[Insert Jen's Mockups]

Changes

  • Adds a solid blue halo around the selected node. The halo is "outside" the progress bar, but "inside" the outline from a draft/review node. The halo should fade in, and animate smoothly when the node radius changes, at the same rate as the node.
    image

Screenshot

image

Issue Linkage

Closes #1249

PR Dependency

Depends on #1190

Automated Testing

N/A

@zifgu zifgu mentioned this pull request Aug 26, 2022
Closed
@zifgu
Copy link
Contributor Author

zifgu commented Aug 27, 2022 •
edited
Loading

While testing, I noticed that the node progress bar moves faster at the beginning of the animation than the main node. This was true before this PR, but has become more noticeable with the addition of the selection halo because the different animation rates make a seam appear between the progress bar and selection halo.

Behaviour on radius change, with selection halo:

1249-progress-bar-with-selection-halo.mp4

Behaviour on radius change, without selection halo:

1249-progress-bar-without-selection-halo.mp4

Nevertheless, as a cosmetic change only, fixing this may not be a high priority.

@wynnset wynnset changed the title 1249 - Use halo to indicate selected node rather than size change 1249 - Tapestry 3.0 - Use halo to indicate selected node rather than size change Sep 6, 2022
legendword
legendword approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@legendword legendword left a comment

Choose a reason for hiding this comment

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

The code change looks great and the functionality is working perfectly.

@legendword legendword added the needs final review Has been tested and reviewed once and needs another code review to become ready for merge label Sep 8, 2022
@cypress
Copy link

cypress bot commented Oct 21, 2022 •
edited
Loading


Test summary

95 • 0 • 0 • 0 • Flakiness 0

Run details
Project tapestry-wp
Status Passed
Commit cf84d7b
Started Nov 27, 2022 12:45 AM
Ended Nov 27, 2022 12:49 AM
Duration 04:09 💡
OS Linux Ubuntu - 22.04
Browser Chrome 107

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@legendword legendword linked an issue Oct 31, 2022 that may be closed by this pull request
Use halo to indicate selected node rather than size change #1249
Open
@legendword legendword merged commit 5575c31 into 1190-zoomable-interface Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legendword legendword legendword approved these changes

Assignees

@wynnset wynnset

Labels
needs final review Has been tested and reviewed once and needs another code review to become ready for merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use halo to indicate selected node rather than size change
3 participants
@zifgu @legendword @wynnset