FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Added Light and Dark Grey Main and Secondary themes as per Request #2600

Open fdarcey opened this issue 9 months ago • 20 comments

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

image

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

fdarcey avatar Nov 07 '23 19:11 fdarcey

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.

absidue avatar Nov 07 '23 20:11 absidue

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)

kommunarr avatar Nov 28 '23 19:11 kommunarr

To clarify, I'm referring to updating this section of the code: Screenshot_20231128_145920

And the other one corresponding to sec colors

kommunarr avatar Nov 28 '23 21:11 kommunarr

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: Screenshot_20231128_151840

and the secondary color class to this selector: Screenshot_20231128_151944

kommunarr avatar Nov 28 '23 21:11 kommunarr

@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.

kommunarr avatar Nov 30 '23 21:11 kommunarr

@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.

I think I already did that, as shown here:

lightdarkmain lightdarsec

Is there something wrong here?

fdarcey avatar Dec 05 '23 19:12 fdarcey

@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.

kommunarr avatar Dec 05 '23 22:12 kommunarr

Sorry for vanishing a few weeks, the next update is coming soon.

fdarcey avatar Dec 22 '23 00:12 fdarcey

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.

fdarcey avatar Dec 22 '23 00:12 fdarcey

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. Screenshot_20240102_132156

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. Screenshot_20240102_132301

kommunarr avatar Jan 02 '24 19:01 kommunarr

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.

github-actions[bot] avatar Feb 03 '24 01:02 github-actions[bot]

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.

fdarcey avatar Feb 10 '24 03:02 fdarcey

@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.

github-actions[bot] avatar Mar 30 '24 01:03 github-actions[bot]

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).

kommunarr avatar Apr 17 '24 19:04 kommunarr

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.

kommunarr avatar Apr 17 '24 20:04 kommunarr

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.

github-actions[bot] avatar May 16 '24 01:05 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar May 16 '24 16:05 github-actions[bot]

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.

github-actions[bot] avatar Jun 18 '24 01:06 github-actions[bot]

This PR was closed because it has been stalled for 14 days with no activity.

github-actions[bot] avatar Jul 02 '24 01:07 github-actions[bot]