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

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

Open
wants to merge 16 commits into
base: development
Choose a base branch
from

Conversation

fdarcey
Copy link

@fdarcey fdarcey commented Nov 7, 2023

Title

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

Pull Request Type

  • Bugfix
  • 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

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 7, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 7, 2023 19:58
auto-merge was automatically disabled November 7, 2023 20:09

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 7, 2023 20:09
@absidue
Copy link
Member

absidue commented Nov 7, 2023

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.

src/renderer/themes.css Outdated Show resolved Hide resolved
static/locales/en-US.yaml Outdated Show resolved Hide resolved
auto-merge was automatically disabled November 9, 2023 20:42

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 9, 2023 20:43
@efb4f5ff-1298-471a-8973-3d47447115dc

I would suggest changing everything to gray instead of grey just to avoid confusion in the code

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 12, 2023
auto-merge was automatically disabled November 14, 2023 19:18

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 14, 2023 19:18
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Nov 14, 2023
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@kommunarr kommunarr left a comment

Choose a reason for hiding this comment

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

suggestion (blocking): --text-with-accent-color (as secondary) and --text-with-main-color (as primary) for dark grey needs to be white (#FFFFFF) or another light color to resolve the issue of inaccessible color contrast with complementing text.

Screenshot_20231122_115203

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 22, 2023
Copy link
Contributor

github-actions bot commented Feb 3, 2024

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.

auto-merge was automatically disabled February 10, 2024 03:17

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) February 10, 2024 03:17
@fdarcey
Copy link
Author

fdarcey commented Feb 10, 2024

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.

@efb4f5ff-1298-471a-8973-3d47447115dc

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

Copy link
Contributor

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.

@kommunarr
Copy link
Collaborator

kommunarr commented Apr 17, 2024

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

@@ -566,6 +582,26 @@ it can be safely elided. This looks quite pleasant on this theme. */
.secDeepOrange {
--text-with-accent-color: #000000;
}
.secLightGray {
--text-with-accent-color: #000000;
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.svg");

.secLightGray {
--text-with-accent-color: #000000;
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");
--logo-text-bar-color: url("../../_icons/textBlackSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-text-bar-color: url("../../_icons/textBlackSmall.png");
--logo-text-bar-color: url("../../_icons/textBlackSmall.svg");

}
.secDarkGray {
--text-with-accent-color: #FFFFFF;
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.svg");

.secDarkGray {
--text-with-accent-color: #FFFFFF;
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");
--logo-text-bar-color: url("../../_icons/textWhiteSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-text-bar-color: url("../../_icons/textWhiteSmall.png");
--logo-text-bar-color: url("../../_icons/textWhiteSmall.svg");

}
.mainDarkGray {
--text-with-main-color: #FFFFFF;
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.svg");

.mainDarkGray {
--text-with-main-color: #FFFFFF;
--logo-icon-bar-color: url("../../_icons/iconWhiteSmall.png");
--logo-text-bar-color: url("../../_icons/textWhiteSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-text-bar-color: url("../../_icons/textWhiteSmall.png");
--logo-text-bar-color: url("../../_icons/textWhiteSmall.svg");

.mainLightGray {
--text-with-main-color: #000000;
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");
--logo-text-bar-color: url("../../_icons/textBlackSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-text-bar-color: url("../../_icons/textBlackSmall.png");
--logo-text-bar-color: url("../../_icons/textBlackSmall.svg");

@@ -286,6 +286,22 @@ it can be safely elided. This looks quite pleasant on this theme. */
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");
--logo-text-bar-color: url("../../_icons/textBlackSmall.png");
}
.mainLightGray {
--text-with-main-color: #000000;
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");

Choose a reason for hiding this comment

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

Suggested change
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.png");
--logo-icon-bar-color: url("../../_icons/iconBlackSmall.svg");

@kommunarr
Copy link
Collaborator

kommunarr commented Apr 17, 2024

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.

Copy link
Contributor

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.

Copy link
Contributor

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

@kommunarr kommunarr mentioned this pull request May 21, 2024
1 task
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Grey option for main and secondary color theme
6 participants