plugins: scroll-to functionality

[kecnry]

Description

This pull request implements plugin.open_in_tray(scroll_to=False/True) functionality as well as using this within the plugin shortcut buttons in the viewer toolbar.

Up for discussion:

• should this default to True or False (for the API)?

Screen.Recording.2024-03-27.at.9.08.02.AM.mov

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

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 milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[kecnry]

@mariobuikhuizen - any suggestions to improve this? Right now I handle this by setting a traitlet to True on the python side, and then noticing its true within this callback loop in vue, reverting it to False, and then calling scrollIntoView. I'm not too worried about the potential slight lag since I think that actually helps show the user where the item came from, but this does result in the element being pulled all the way to the top of the browser page, not just within the parent scrolling element.

[kecnry]

Screen.Recording.2024-03-26.at.1.58.18.PM.mov

[kecnry]

ah block: "nearest" helps, but then scrolls to the bottom of the plugin when it needs to scroll up instead of down 🤔

[mariobuikhuizen]

I can reproduce that behaviour in Firefox, chromium doesn't have this.

This would fix it, although it might be a bit brittle to rely on the .overflow-y-auto class, which might be susceptible to changes without consideration for its usage here:

const viewport = this.$el.closest(".overflow-y-auto")
const scrollTarget = this.$el.closest(".v-expansion-panel")
viewport.scrollTo({top: scrollTarget.offsetTop, behavior: "smooth"})
// this.$el.scrollIntoView({behavior: "smooth", block: "nearest", inline: "start"});

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 88.72%. Comparing base (f1ba4a6) to head (c3e2ed7).
Report is 6 commits behind head on main.

❗ Current head c3e2ed7 differs from pull request most recent head 481bf30. Consider uploading reports for the commit 481bf30 to get more accurate results

Files	Patch %	Lines
jdaviz/core/template_mixin.py	60.00%	2 Missing ⚠️
jdaviz/core/tools.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2768      +/-   ##
==========================================
- Coverage   88.72%   88.72%   -0.01%     
==========================================
  Files         110      110              
  Lines       16353    16367      +14     
==========================================
+ Hits        14509    14521      +12     
- Misses       1844     1846       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bmorris3]

should this default to True or False (for the API)?

I'm in favor of default True for the API. There's ~no point in opening a plugin in the tray without being able to see it.

[kecnry]

There's ~no point in opening a plugin in the tray without being able to see it.

The one case I can think of is when opening multiple plugins from a single cell, but agreed that defaulting to True still makes sense.

[gibsongreen]

I support this to defaulting to True, and I can't wait for this to be on main (so many clicks saved!)

[rosteen]

I support this to defaulting to True, and I can't wait for this to be on main (so many clicks saved!)

Agreed on both counts, this is really nice even if not quite perfect.