jdaviz icon indicating copy to clipboard operation
jdaviz copied to clipboard

Disable cloning viewers in specviz and specviz2d

Open MatthewPortman opened this issue 1 month ago • 3 comments

Description

This pull request disables cloning viewer tabs in specviz and specviz2d to avoid tab management issues since tabs are disabled for these configs.

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?
  • [ ] If new remote data is added that uses MAST, is the URI added to the cache-download.yml workflow?
  • [ ] 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)?

MatthewPortman avatar Nov 04 '25 22:11 MatthewPortman

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 88.14%. Comparing base (648148a) to head (3745dc0). :warning: Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3876   +/-   ##
=======================================
  Coverage   88.14%   88.14%           
=======================================
  Files         194      194           
  Lines       26625    26627    +2     
=======================================
+ Hits        23469    23471    +2     
  Misses       3156     3156           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 04 '25 22:11 codecov[bot]

This gets into trouble as there is no way in the configs to restore closed viewers. If we enable tab headers, we must use css to hide the close button on at least some viewers to prevent this situation.

Which viewers? I haven't worked with these much but I'm guessing mosviz and maybe rampviz?

Edit: I just reread your response in the ticket. So your idea is to hide the close button on all viewers that don't have the ability to create a new viewer like specviz, until a second viewer is created.

MatthewPortman avatar Nov 05 '25 14:11 MatthewPortman

I think it'll probably be much simpler to just disable cloning in configs.... but yes, there is already some css I think for Imviz that prevents closing either the original viewer or the last remaining viewer.

kecnry avatar Nov 05 '25 14:11 kecnry

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v4.4.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 964f372be83fa2ac15858cc1c803a074d89d7694
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #3876: Disable cloning viewers in specviz and specviz2d'
  1. Push to a named branch:
git push YOURFORK v4.4.x:auto-backport-of-pr-3876-on-v4.4.x
  1. Create a PR against branch v4.4.x, I would have named this PR:

"Backport PR #3876 on branch v4.4.x (Disable cloning viewers in specviz and specviz2d)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

lumberbot-app[bot] avatar Nov 12 '25 15:11 lumberbot-app[bot]