FreeTube
FreeTube copied to clipboard
Added Light and Dark Grey Main and Secondary themes as per Request #2600
Title
Added Light and Dark Grey Main and Secondary themes as per Request #2600
Pull Request Type
- [ ] Bugfix
- [x] Feature Implementation
- [ ] Documentation
- [ ] Other
Related issue
closes #2600
Description
FreeTube/src/renderer/themes.ccs now has:
- .mainLightGrey with primary-color: #B1B1B1, primary-color-hover: #848482, and primary-color-active: #7E7E7E. Lines 266, 365-369.
- .mainDarkGrey with primary-color: ##696969, primary-color-hover: #555555, and primary-color-active: #4E4E4E. Lines 267, 371-375.
- .secLightGrey with primary-color-active: #4E4E4E, accent-color-hover: #848482, accent-color-active: #7E7E7E, accent-color-light: #cfcfcf, accent-color-visited: #797979, accent-color-opacity1: rgba(177,177,177,0.04), with accent-color-opacity2, 3, and 4 having their fourth value at 0.12, 0.16, and 0.24, respectively. Lines 561, 756-766.
- .secDarkGrey with accent-color: #696969, accent-color-hover: #555555, accent-color-active: #4E4E4E, accent-color-light: #A5A5A5, accent-color-visited: #444444, accent-color-opacity1: rgba(105,105,105,0.04), with accent-color-opacity2, 3, and 4 having their fourth value at 0.12, 0.16, and 0.24, respectively. Lines 562, 768-778.
FreeTube/src/renderer/helpers/colors.js has:
- { name: 'LightGrey', value: '#B1B1B1'}, and { name: 'DarkGrey', value: '#696969'} on line 18 and 19 under export const colors.
FreeTube/static/locales/en-US.yaml has:
- Light Grey: Light Grey and Dark Grey: Dark Grey on line 230 and 231.
Overall, the user could now select Light Grey and Dark Grey for main and secondary themes and have the corresponding UI elements switch to those colors. Note: I have only updated en-US.yalm with Light and Dark Grey, deciding to allow the users more experienced with translations add in such for other languages. This means that until then, the main and secondary color menus will show them with "Setting.Theme Settings." preceding them.
Screenshots
Testing
This has been tested, and the result is shown in the screenshot.
Desktop
- OS: Windows
- OS Version: 11
- FreeTube version: 0.19.1 Beta
Additional context
Issues are considered closed when the feature has been implemented or bug fixed. If we waited for all translators to log into weblate and translate stuff before closing issues, we would have them open for months or longer.
I would suggest changing everything to gray instead of grey just to avoid confusion in the code
nitpick: Can you group the text-with
colors with the rest on lines 240 and 537? (as in, put the .mainDarkGray
and .secDarkGray
classes in those places, and remove lines 769 and 372)
To clarify, I'm referring to updating this section of the code:
And the other one corresponding to sec colors
You also need to set an explicit text color of black for the text color of light gray by adding the class to this selector:
and the secondary color class to this selector:
@fdarcey Thanks for adding the light gray changes! Could you also move these text-with-main-color
and text-with-accent-color
parts grouped with the other classes in the comma-delimited selectors that I mention here and here? As in, for example, making this change:
.mainDarkGray,
.mainRed,
.mainPink,
.mainPurple,
.mainDeepPurple,
.mainIndigo,
.mainBlue,
.mainLightBlue,
.mainCyan,
.mainTeal,
.mainGreen {
--text-with-main-color: #FFFFFF;
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");
--logo-text-bar-color: url("../../_icons/textWhiteSmall.png");
}
And doing the same for the proper places for secDarkGray, mainLightGray, and secLightGray.
@fdarcey Thanks for adding the light gray changes! Could you also move these
text-with-main-color
andtext-with-accent-color
parts grouped with the other classes in the comma-delimited selectors that I mention here and here? As in, for example, making this change:.mainDarkGray, .mainRed, .mainPink, .mainPurple, .mainDeepPurple, .mainIndigo, .mainBlue, .mainLightBlue, .mainCyan, .mainTeal, .mainGreen { --text-with-main-color: #FFFFFF; --logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png"); --logo-text-bar-color: url("../../_icons/textWhiteSmall.png"); }
And doing the same for the proper places for secDarkGray, mainLightGray, and secLightGray.
I think I already did that, as shown here:
Is there something wrong here?
@fdarcey Oh yes, to clarify, didn't mean to say that you didn't have the code present. I'm just saying to do it in that manner I proposed, because doing it the other way also misses things like the --logo-icon-bar-color
and --logo-text-bar-color
that should be updated for these theme colors as well. That, and it allows us to more easily update the behaviors for these rules all at once if we need to change them or insert additional behaviors.
Sorry for vanishing a few weeks, the next update is coming soon.
I added the "--logo-icon-bar-color" and "--logo-text-bar-color" to the secondary grays to, I didn't see them associated with other secondary colors, but if they're not supposed to be there than it'd be simpler to remove them than add them.
Thanks for your patience @fdarcey. Final asks:
Please just add .mainLightGray
and .mainDarkGray
to the top two sections instead of giving them their own sections. The reason is that code duplication can lead to updating issues down the road.
Similarly, do the same for the secondary light and dark gray colors, adding them to the top two sections in this image & removing their own duplicated sections below. Those duplicated sections also modify the logo colors, which is incorrect.
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.
First of all, I'm sorry for forgetting about this. The schedule which once had a specific time I went to work on this project changed, and as a result other thinks took more of my mental space.
Secondly, I'm afraid that I don't understand my last task. I tried to fulfill it as best as I could interpret it, but I don't know if it was correct.
@jasonhenriquez
First of all, I'm sorry for forgetting about this. The schedule which once had a specific time I went to work on this project changed, and as a result other thinks took more of my mental space.
Secondly, I'm afraid that I don't understand my last task. I tried to fulfill it as best as I could interpret it, but I don't know if it was correct.
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.
Hi @fdarcey, I apologize for the communication troubles. I've created this PR here with a commit for implementing the suggested changes (& resolving merge conflicts).
To clarify, the above issues detected are merge conflict issues not detected by the GH action, and these are all resolved in my commit above.
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This PR was closed because it has been stalled for 14 days with no activity.