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

feat(core/managed): scroll to selected version in sidebar, update scroll containers #8618

Merged
merged 4 commits into from
Oct 5, 2020
Merged

feat(core/managed): scroll to selected version in sidebar, update scroll containers #8618

merged 4 commits into from
Oct 5, 2020

Conversation

erikmunson
Copy link
Member

Boy was this some "fun". Couple changes:

  • Add some pretty 🙄 logic for scrolling to the currently selected version in the sidebar. This is especially helpful when clicking on a link to a version that's not in the most recent ~5-10, or in similar situations like clicking on the new "another version is pinned" warning to jump you to a different version. I made an explicit decision to go with the lowest-effort approach here because I expect this code to change very quickly and didn't want to invest in optimizations yet. I have expectations that we'll move to paging + virtualization in the foreseeable future, and at that point leaning on the virtualization library for this will probably make sense
  • As part of doing the first thing, I tripped over a thing I've been putting off forever: the scroll containers on the list/detail layout are just terrible and I should be ashamed. This was mostly just causing weird visual behavior when scrollbars were shown up until now, but this change forced some adjustments that made it critical to refactor the scroll containers. I've taken the opportunity to not just adjust the containers functionally, but also tweak the layout slightly to make scrollbars jive aesthetically with the content more as well. Screenshots below.
Sidebar

Before
Screen Shot 2020-10-02 at 2 59 58 PM (2)

After
Screen Shot 2020-10-02 at 2 23 52 PM (2)

Overview

Before
Screen Shot 2020-10-02 at 3 02 01 PM (2)

After
Screen Shot 2020-10-02 at 2 34 37 PM (2)

Version details

Before
Screen Shot 2020-10-02 at 3 02 45 PM (2)

After
Screen Shot 2020-10-02 at 3 03 36 PM (2)

cc @gcomstock

gcomstock
gcomstock reviewed Oct 2, 2020
View reviewed changes
app/scripts/modules/core/src/managed/ArtifactsList.tsx

!isInView && rowRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' });
}
}, 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

hyuk

@erikmunson erikmunson merged commit 815742d into spinnaker:master Oct 5, 2020
@erikmunson erikmunson deleted the md-env-scroll-to-selected-version branch October 5, 2020 20:53
erikmunson pushed a commit that referenced this pull request Oct 5, 2020
chore(core): publish core@0.0.516
f36e254 
80858e7 feat(core/managed): limit artifact versions to 30 (#8623)
90a2819 feat(core/managed): put constraints in a 'skipped' state on skipped versions (#8620)
815742d feat(core/managed): scroll to selected version in sidebar, update scroll containers (#8618)
de24ef3 feat(core/managed): apply consistent sorting to resources (#8622)
3735483 fix(core/pipeline): Always enable "show revision history" in edit pipeline dialog (#8621)
abffd84 fix(core/managed): fixup some details from new layout (#8619)
f1bb04e fix(appname): encodeURIComponent for app name (#8586)
60c0c7b feat(validation): Allow app name validators to be overridden (#8584)
@erikmunson erikmunson mentioned this pull request Oct 5, 2020
Merged
mergify bot pushed a commit that referenced this pull request Oct 5, 2020
chore(package): amazon@0.0.269 azure@0.0.255 cloudfoundry@0.0.101 cor…
8aa1e3e 
…e@0.0.516 docker@0.0.60 google@0.0.21 oracle@0.0.9 tencentcloud@0.0.6 (#8624)

* chore(amazon): publish amazon@0.0.269

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(azure): publish azure@0.0.255

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(cloudfoundry): publish cloudfoundry@0.0.101

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(core): publish core@0.0.516

80858e7 feat(core/managed): limit artifact versions to 30 (#8623)
90a2819 feat(core/managed): put constraints in a 'skipped' state on skipped versions (#8620)
815742d feat(core/managed): scroll to selected version in sidebar, update scroll containers (#8618)
de24ef3 feat(core/managed): apply consistent sorting to resources (#8622)
3735483 fix(core/pipeline): Always enable "show revision history" in edit pipeline dialog (#8621)
abffd84 fix(core/managed): fixup some details from new layout (#8619)
f1bb04e fix(appname): encodeURIComponent for app name (#8586)
60c0c7b feat(validation): Allow app name validators to be overridden (#8584)

* chore(docker): publish docker@0.0.60

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(google): publish google@0.0.21

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(oracle): publish oracle@0.0.9

f1bb04e fix(appname): encodeURIComponent for app name (#8586)

* chore(tencentcloud): publish tencentcloud@0.0.6

f1bb04e fix(appname): encodeURIComponent for app name (#8586)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gcomstock gcomstock gcomstock left review comments

@vigneshm vigneshm Awaiting requested review from vigneshm

Assignees
No one assigned
Labels
target-release/1.23
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erikmunson @gcomstock @spinnakerbot