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.

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

fix advance color bugs #940

Merged
merged 2 commits into from
Jul 15, 2020
Merged

fix advance color bugs #940

merged 2 commits into from
Jul 15, 2020

Conversation

andreicristianpetcu
Copy link
Contributor

@andreicristianpetcu andreicristianpetcu commented Jul 8, 2020 •
edited

Fixes #936.
Fixes #934.

@andreicristianpetcu andreicristianpetcu force-pushed the advance_colors_fixes branch 2 times, most recently from aa68829 to b397d9a Compare July 8, 2020 20:26
@andreicristianpetcu andreicristianpetcu changed the title Fix #936 override tab_line only if missing fix advance color bugs Jul 8, 2020
Rob--W
Rob--W requested changes Jul 8, 2020
View reviewed changes
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

I saw the issue before the PR, so I commented here #934 (comment) . Please update the patch with that and the comment below.

src/lib/themes.js Outdated Show resolved Hide resolved
@Rob--W
Copy link
Member

Rob--W commented Jul 8, 2020

PS. If you edit the PR and add the magic keywords

Fixes #936.
Fixes #934.

... then the PR would automatically be linked to those issues (which helps with visibility), and those issues would automatically be closed when the PR is merged.

Rob--W
Rob--W requested changes Jul 9, 2020
View reviewed changes
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Thanks, this looks a bit better. After having followed my suggestion of trimming the menu label, the height can be even smaller. Let's do that (to minimize the impact on the "Custom Colors") tab.

I'll merge after the following changes have been made.

src/web/lib/components/StorageSpaceInformation/index.scss Outdated Show resolved Hide resolved
src/web/lib/components/ThemeBuilder/index.scss Outdated Show resolved Hide resolved
src/web/lib/components/ThemeColorsEditor/index.scss Outdated Show resolved Hide resolved
@andreicristianpetcu
Copy link
Contributor Author

Done @Rob--W 🍾

@andreicristianpetcu
Copy link
Contributor Author

andreicristianpetcu commented Jul 9, 2020 •
edited

Done 🚀

Rob--W
Rob--W approved these changes Jul 9, 2020
View reviewed changes
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

LGTM thanks. I'll check this again tomorrow and try to create a release next week.

@Rob--W Rob--W merged commit 523f47d into mozilla:master Jul 15, 2020
@andreicristianpetcu andreicristianpetcu deleted the advance_colors_fixes branch July 15, 2020 20:44
@andreicristianpetcu
Copy link
Contributor Author

Any update with the release @Rob--W ?

@Rob--W
Copy link
Member

Rob--W commented Jul 19, 2020

The add-on is in the review queue on AMO. Once approved, I'll publish the latest version of the site. This will all happen within a few days.

@andreicristianpetcu
Copy link
Contributor Author

Thank you for investing all this time in a project that is basically in maintenance. I really appreciate it! ❤️ @Rob--W

@Rob--W
Copy link
Member

Rob--W commented Jul 22, 2020

Version 2.1.6 of the add-on has been approved, and I just updated the website to match. I did a quick check and it seems to be working as intended.

@andreicristianpetcu
Copy link
Contributor Author

Once I started to port my themes to FF color I started to see issues :)))
I did not add ARGB for the ones that I needed to....
popup_highlight
sidebar_highlight_text
sidebar_text
toolbar_field_separator
toolbar_field_text_focus
toolbar_top_separator
How my theme looks
image
http://localhost:8080/?theme=XQAAAAKmBAAAAAAAAABBKYhm849SCicxcUhCHjNpdzzyWMw6jC7AjsFFvBq6bJb_pTIEztZDnJzUQhD-FvSzC0EJgjLntIeQ3eFXB2dFAz5qr7nage4lSjQN8HT2PztF1_2o_8cLNvsqO6kPj4elEC0VG4hnOYqIV3ZQZqvAHslb9NVQNZ9oCyZ7t_cEuONPMx_grMMZ6fZBR0EmzF2Czi1qMurGq8wWTy-SkNcJIslJjH3vhFgRBXbMT--VDouoTaCzVi1GisAPtE3xMpZS9eGwnmjzjFvYHa_dQzf2kKnYgYhoTZvpEWTYZkFtMIJOrgc58qEfusdpN28rXJVRyz3jLJQ-11hf9WCyhWDX-oYp7NKByOjg61gVWqrr8utj1OIVC4To7Ef4S8i7LlFZe1GwHDe9GgCM_pj5VOXACkimNFqDbrNiJK-3o-8NJP1aiVr4uXhgw2NN63DU6nc5Qt9GPJ5iut-hbhzfTHzZiwTnJ6_267IF3e2BEVsLNZSt5hU-1Cb_WdosFkjuM8BhK0xHIMMSi5FVbhxQsPhtKng_eiQgOm2VGH8W7x0t5l_9BrcO

How it should look. https://addons.mozilla.org/en-US/firefox/addon/borderless-dark/?src=search
image

@andreicristianpetcu
Copy link
Contributor Author

I got a close aproximation to my theme without alpha. Maybe I'll send in another PR soon 😄
image
https://color.firefox.com/?theme=XQAAAAJuBAAAAAAAAABBKYhm849SCicxcUhA3DJozHnOMuprjJIF9jJb-FWu-LKNPPR7QEVwEchbrYDpU0TML3WJHDuqqbxDwW31jv3GZ5hvMvMhiQXynt-0bl-j-9cLJkvthNcb0jrhLDSYQOJFf0FV3Dw3wpXVGP5f5N-DX26ctWmKCGwwgzAjqRhHzvajRZ6XAw2FLmknF51JbIUfcgP3TNJ8zvVJCmjYH1Db-VxCiQG3XqWv74FyAih4TSDmHZUyS2utJX-ViDEr_NzIxldusOIyuUNh_nKpg-fnjzqr1g68Jj13hlsfNX4uc76NoRIuwNJnhJ4RSGtmNfjzVdL-Ck2SSKkJ_7RMWmfL-rfe6q3FC-JXig1DIRjdJ2HcthUeDZNeIZl6nYPwP4HtRTqo6YAzEWof0mDaqNNGwwZO3nxIVr6FAVWdUcnzuFCsfqwx6XdyZsDaOCAXIO9BLHwg5EHA7q9ZmujQBX8DV5LEeXQLBDWDlrgYuxThoINgvJYpkxzHJushF2yCM5JeNA5A8f1p1cDtdQosRoco2B-nkRzbjdNFnP_jD1BP

@Rob--W Rob--W mentioned this pull request Jul 23, 2020
Closed
@andreicristianpetcu andreicristianpetcu mentioned this pull request Jul 23, 2020
Closed
@Rob--W Rob--W mentioned this pull request Jul 23, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rob--W Rob--W Rob--W approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@andreicristianpetcu @Rob--W