-
Notifications
You must be signed in to change notification settings - Fork 75
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.
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
data-menu: API hints #3274
data-menu: API hints #3274
Conversation
2ff8880
to
90a3fe7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3274 +/- ##
==========================================
- Coverage 88.81% 88.81% -0.01%
==========================================
Files 125 125
Lines 18960 18968 +8
==========================================
+ Hits 16839 16846 +7
- Misses 2121 2122 +1 ☔ View full report in Codecov by Sentry. |
to programmatically open the viewer in any existing viewer instances
I really like it! |
@gibsongreen mentioned suggestions for the icon as well, maybe we can get a custom icon from @Jenneh for that down the road. I couldn't find anything off-the-shelf as a single icon that worked well 🫤 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the code changes look good to me and I really like the hints at the app-level!
I also tried clicking on the lock and agree about the tooltip, but got used to it so I’m for whichever path forward works best.
The other icon suggestion was in a followup effort to possibly update the < >
to <API>
. As long as the width works out I think it would add a lot of clarity.
This looks good! Is this not complete for all plugins yet? I don't see hints for aperture photometry |
This PR only brings to the data-menu. Previous PRs have exposed hints for all plugins with public APIs (aperture photometry is not one of those yet). |
Description
This pull request moves API hint toggles from the plugin to the app-level and implements API hints for the data-menu.
Screen.Recording.2024-11-08.at.3.24.35.PM.mov
Change log entry
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, maintainershould 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.
trivial
label.