Skip to content

vs/refactor-themes #818

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 2, 2025
Merged

vs/refactor-themes #818

merged 5 commits into from
Oct 2, 2025

Conversation

vsangereanMOZ
Copy link
Collaborator

Relevant Links

Bugzilla: 1990912
TestRail: N/A

Description of Code / Doc Changes

Refactorting for Themes tests:

  • Promoted dictionaries and flags to ALL_CAPS constants
  • Added concise docstrings for clarity
  • Improved colors_match for safety (handles rgb/rgba, div-by-zero)

Thank you!

Tracy-Walker
Tracy-Walker previously approved these changes Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@Tracy-Walker Tracy-Walker left a comment

Choose a reason for hiding this comment

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

looks good

soncuteanca
soncuteanca reviewed Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@soncuteanca soncuteanca left a comment

Choose a reason for hiding this comment

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

Looks good, only some minor suggestions.

@vsangereanMOZ
Copy link
Collaborator Author

Addressed reviews.
Improved test_redirect_to_addons test to make sure it does not affect the rest of tests within the file.

@vsangereanMOZ
Copy link
Collaborator Author

Add a new set of colours for themes.
Darker colors are different when focus is there or not.

@vsangereanMOZ
Copy link
Collaborator Author

Ok done.
Only good reviews allowed from now on 🗡️

soncuteanca
soncuteanca approved these changes Oct 2, 2025
View reviewed changes
Copy link
Collaborator

@soncuteanca soncuteanca left a comment

Choose a reason for hiding this comment

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

Looks good.

@vsangereanMOZ vsangereanMOZ merged commit b3aa98c into main Oct 2, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soncuteanca soncuteanca soncuteanca approved these changes

@ben-c-at-moz ben-c-at-moz Awaiting requested review from ben-c-at-moz

@sv-hyacoub sv-hyacoub Awaiting requested review from sv-hyacoub

@Tracy-Walker Tracy-Walker Awaiting requested review from Tracy-Walker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vsangereanMOZ @Tracy-Walker @soncuteanca