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

gui/experiments: add custom colors for experiment windows #2590

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

fsagbuya
Copy link
Contributor

@fsagbuya fsagbuya commented Oct 9, 2024

ARTIQ Pull Request

Description of Changes

Adds support for custom title bar and windows color in the experiment windows. Users can now select, apply, and reset these colors from the context menu:

  • Color handling added to _ArgumentEditor and _ExperimentDock to apply colors to widgets.
  • Introduces _ThemedTitleBar style to override the MDISubWindow title bar.
  • ExperimentManager manages color storage and retrieval, persisting them across sessions.
  • Updated state save/restore to include colors.

TODO: clean up storage when experiments get deleted (Other PR)

Color Menu
image

Title Bar
image

Window
image

Dark Theme Example
image

Windows Minimized
image

Related Issue

#2576

Type of Changes

Type
✨ New feature

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

  • Use correct spelling and grammar.
  • Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.
  • Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

  • Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
  • Test your changes or have someone test them. Mention what was tested and how.
    • nix shell to run artiq unit tests
    • basic functionality
  • Add and check docstrings and comments

Documentation Changes

  • Check, test, and update the documentation in doc/. Build documentation (nix build .#artiq-manual-html; nix build .#artiq-manual-pdf) to ensure no errors.

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@sbourdeauducq
Copy link
Member

This appears to break the greyed default value in empty text entry boxes. Here is what it should look like:
image

@fsagbuya
Copy link
Contributor Author

fsagbuya commented Oct 9, 2024

This appears to break the greyed default value in empty text entry boxes.

Updated. Here's the new look:
image
image

@dhslichter
Copy link
Contributor

Would it be possible/straightforward to have the color highlighting only applied to the title bar, or to be able to choose different colors for the title bar and the rest of the window?

@fsagbuya
Copy link
Contributor Author

Would it be possible/straightforward to have the color highlighting only applied to the title bar, or to be able to choose different colors for the title bar and the rest of the window?

Yes. See updated description.

@fsagbuya fsagbuya force-pushed the experiment-themes branch 6 times, most recently from 1cc9b26 to 0a2a3ca Compare October 18, 2024 04:51
@sbourdeauducq
Copy link
Member

Looks like the colors[expurl] dictionary accumulates entries forever and is not cleared when experiments are removed?
What would be a good way to clean it up?

@fsagbuya fsagbuya force-pushed the experiment-themes branch 2 times, most recently from dcae175 to 3816c99 Compare October 22, 2024 07:57
def save_state(self):
self.cleanup_state()
Copy link
Member

Choose a reason for hiding this comment

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

Sounds very hacky (and potentially leading to obscure behavior) that state only gets cleaned up at this point.

for attr in attributes:
state_dict = getattr(self, attr)
for expurl in list(state_dict.keys()):
if expurl[:5] == "repo:" and expurl[5:] not in self.explist:
Copy link
Member

Choose a reason for hiding this comment

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

Please explain the state cleanup strategy in source code comments.

Copy link
Member

Choose a reason for hiding this comment

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

Considering the cases of experiments inside and outside repos.

@fsagbuya fsagbuya changed the title gui/experiments: add custom color themes for experiment windows gui/experiments: add custom colors for experiment windows Nov 12, 2024
@sbourdeauducq sbourdeauducq merged commit 52c07a2 into m-labs:master Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants