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

[Bug]: style - button hover background colors #1971

Closed
1 task done
MatissJanis opened this issue Nov 25, 2023 · 9 comments · Fixed by #2123
Closed
1 task done

[Bug]: style - button hover background colors #1971

MatissJanis opened this issue Nov 25, 2023 · 9 comments · Fixed by #2123
Labels
bug Something isn't working good first issue Good for newcomers user interface Related to the user interface

Comments

@MatissJanis
Copy link
Member

MatissJanis commented Nov 25, 2023

Verified issue does not already exist?

  • I have searched and found no existing issue

What happened?

Somewhere along the way to darkmode we have lost hover colors for most buttons in light mode. We should bring them back (maybe not the exact colors as before if they violate the color paletter, but still something)

We are missing hover color for the primary color buttons. For example: rule creation.

What error did you receive?

No response

Where are you hosting Actual?

None

What browsers are you seeing the problem on?

No response

Operating System

None

@MatissJanis MatissJanis added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed user interface Related to the user interface regression This bug appeared after a recent release and needs to be fixed before the next release labels Nov 25, 2023
@carkom
Copy link
Contributor

carkom commented Nov 26, 2023

Can you provide an example?

@MatissJanis
Copy link
Member Author

Just checked v23.7.0. Seems the problem is there too. So it doesn't seem to be a regression after all. Just something we've never had I guess. Weird either way.

Button Hovered
Screenshot 2023-11-26 at 15 41 52 Screenshot 2023-11-26 at 15 41 52

@MatissJanis MatissJanis removed the regression This bug appeared after a recent release and needs to be fixed before the next release label Nov 26, 2023
@carkom
Copy link
Contributor

carkom commented Nov 26, 2023

Good news is that it's really easy to change. It's already assigned in "Button.tsx"

const backgroundColorHover = {
normal: theme.buttonNormalBackgroundHover,
primary: theme.buttonPrimaryBackgroundHover,
bare: theme.buttonBareBackgroundHover,
menu: theme.buttonMenuBackgroundHover,
menuSelected: theme.buttonMenuSelectedBackgroundHover,
link: theme.buttonBareBackground,
};

Only change happens in "light.ts" where background and backgroundhover are the same.

export const buttonPrimaryText = colorPalette.white;
export const buttonPrimaryTextHover = buttonPrimaryText;
export const buttonPrimaryBackground = colorPalette.purple500;
export const buttonPrimaryBackgroundHover = buttonPrimaryBackground;
export const buttonPrimaryBorder = buttonPrimaryBackground;

@Ife-Ody
Copy link
Contributor

Ife-Ody commented Dec 16, 2023

Is anyone already working on this? I am looking to make some contributions because I love the vision

@carkom
Copy link
Contributor

carkom commented Dec 16, 2023

Not that I'm aware of. Feel free. Please tag me for review.

@Ife-Ody
Copy link
Contributor

Ife-Ody commented Dec 23, 2023

finally working on this today

@Ife-Ody
Copy link
Contributor

Ife-Ody commented Dec 23, 2023

The fix was easy. I just picked a shade lighter in the light mode and a shade darker in the dark mode

Good news is that it's really easy to change. It's already assigned in "Button.tsx"

const backgroundColorHover = { normal: theme.buttonNormalBackgroundHover, primary: theme.buttonPrimaryBackgroundHover, bare: theme.buttonBareBackgroundHover, menu: theme.buttonMenuBackgroundHover, menuSelected: theme.buttonMenuSelectedBackgroundHover, link: theme.buttonBareBackground, };

Only change happens in "light.ts" where background and backgroundhover are the same.

export const buttonPrimaryText = colorPalette.white; export const buttonPrimaryTextHover = buttonPrimaryText; export const buttonPrimaryBackground = colorPalette.purple500; export const buttonPrimaryBackgroundHover = buttonPrimaryBackground; export const buttonPrimaryBorder = buttonPrimaryBackground;

@carkom
Copy link
Contributor

carkom commented Dec 23, 2023

Great! Yea, we implemented the code like that to make color changes really easy. Go ahead and create a PR with your changes. Cheers!

@carkom
Copy link
Contributor

carkom commented Dec 24, 2023

fixed by #2123

carkom added a commit that referenced this issue Feb 2, 2024
… theme #1971 (#2123)

* Bugfix: Add Primary Button hover background colors

* Add release notes

* Rename 1971.md to 2123.md

* Update release note 2123.md typo

* Update packages/desktop-client/src/style/themes/dark.ts

* Update packages/desktop-client/src/style/themes/light.ts

---------

Co-authored-by: Neil <[email protected]>
@github-actions github-actions bot removed the help wanted Extra attention is needed label Feb 2, 2024
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this issue Mar 7, 2024
… theme actualbudget#1971 (actualbudget#2123)

* Bugfix: Add Primary Button hover background colors

* Add release notes

* Rename 1971.md to 2123.md

* Update release note 2123.md typo

* Update packages/desktop-client/src/style/themes/dark.ts

* Update packages/desktop-client/src/style/themes/light.ts

---------

Co-authored-by: Neil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers user interface Related to the user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants