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.

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

Move user-facing information to top of preferences and fix autogenerated UI images #613

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TimMonko
Copy link
Contributor

@TimMonko TimMonko commented Mar 3, 2025

References and relevant issues

Description

  1. Moves user-facing information to the top of the autogenterated preferences document.
  2. Removes the redundant programmatic settings code block (unless we also want to show the settings.SETTINGS version. I think as a learner, understanding how get_settings() is used to set a singleton was more important to my understanding)
  3. Fixes/updates the autogenerated preferences dialog images to correctly match the header to the image, and update the UI to correctly show the selection in the list on the left.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 3, 2025
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thanks @TimMonko for another user-centered improvement. I love the user "how-to" before the preferences reference section. 💐

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Mar 4, 2025

I noticed the screenshots dont match the headings, e.g. Application heading screenshot shows Appearance. I assume something was changed and this was never updated.
But it's that way on live too.

Is there an easy way to get the screenshots in each Section?
So e.g.
Section > Application
Screenshot of what it looks like
each setting listed
Section > Appearance
Screenshot of what it looks like
each setting listed

@psobolewskiPhD
Copy link
Member

Going to link this issue: #13
This PR definitely helps address it, though maybe not close it outright.

@TimMonko
Copy link
Contributor Author

TimMonko commented Mar 4, 2025

Thanks for catching @psobolewskiPhD

This is the code the makes the images:

    for idx, (name, field) in enumerate(NapariSettings.__fields__.items()):
        pref._stack.setCurrentIndex(idx)
        pixmap = pref.grab()
        title = field.field_info.title or name
        pixmap.save(str(IMAGES_PATH / f"preferences-{title.lower()}.png"))

Intuitively, it looks like it should be setting the file names correctly (the section header ends up as title.lower() of the image path), so not sure what is happening. I can try to look into it tonight.

And as for the adding the screenshots to each section, I think that's doable, but I'm not sure if that is good value. Ultimately, I think like #13 we need to separate the user-facing component from the 'dev'-facing part of this, but that's a much bigger lift.

@TimMonko TimMonko changed the title Move user-facing information to top of preferences Move user-facing information to top of preferences and fix autogenerated UI images Mar 5, 2025
@TimMonko
Copy link
Contributor Author

TimMonko commented Mar 5, 2025

@psobolewskiPhD thanks for the eyes! I've updated it and pretty sure I have it cleaned up now.

  1. Headers correctly match the UI image
  2. the UI image now also updates the selection list on the left (which is kinda important I think to show users how to swap between em)

@psobolewskiPhD
Copy link
Member

Nice! you fixed it! 🤩
I still kinda think a layout of
Heading
Screenshot
Listing
could be better?
Like the autogenerated things are still informative of what is in the tab.
image

@psobolewskiPhD
Copy link
Member

This is a huge improvement, so approving.
We can try other layouts in followups.

@psobolewskiPhD psobolewskiPhD added this to the 0.6.0 milestone Mar 5, 2025
@TimMonko
Copy link
Contributor Author

TimMonko commented Mar 5, 2025

Thanks. I understand where your are coming from. I think there are higher value things to work on for now, so we can save for later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants