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

Beautify DTP #2170

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Beautify DTP #2170

wants to merge 8 commits into from

Conversation

Hirnmoder
Copy link

Major changes

  1. App icon highlighting unaffected by hover animation (Resolves "Animate hovering app icons" animates the highlight background of a focused app instead of just the app icon itself #2009)
  2. Allow hover animation travel below 0%
  3. Allow hover animation zoom below 100%
  4. Hover highlighting independent of hover animation
  5. Custom color for hover highlighting
  6. Custom color for mouse down highlighting
  7. Icon background with rounded corners (Resolves Round active application highlight #729 Rounded corners for focused app highlight #1647 Edit corner radius of components #1819)
  8. Customize margin for app icons towards screen border and desktop (Resolves How to add some space between app icons and panel border?  #1721 Edit corner radius of components #1819)

Some visual impressions with custom settings

Change Before After
1 image image
2,3 image image
4 Either animation or highlighting image
5 image image
6 image image
7 image image
8 image image

- Allow hover animation travel below 0%
- Allow hover animation zoom below 100%
- Hover highlighting independent of hover animation
- Custom color for hover highlighting
- Custom color for mouse down highlighting
- Icon background with rounded corners
- Customize margin for app icons towards screen border and desktop
- Allow hover animation travel below 0%
- Allow hover animation zoom below 100%
- Hover highlighting independent of hover animation
- Custom color for hover highlighting
- Custom color for mouse down highlighting
- Icon background with rounded corners
- Customize margin for app icons towards screen border and desktop
@Hirnmoder
Copy link
Author

I've made accidentially a rebase instead of a fast-forward merge. This is why all the other commits show up. Merging should not be affected by this.
However, if you would like me to re-do the PR, just comment and I'll do it.

Copy link

@Arilas Arilas left a comment

Choose a reason for hiding this comment

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

Hey. I've tried to use your PR locally and found some bugs. Some of them are commented here in review.

Also border radius is not working for hovering and active icons. I'm not sure why.

];

if (!this._checkGtkVersion_cssVariables()) {
this._dtpSettingsSignalIds += [
Copy link

Choose a reason for hiding this comment

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

You are converting array into string

Copy link
Author

Choose a reason for hiding this comment

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

I don't see your point here. this._dtpSettingsSignalIds is an array of integers (signal IDs to be precise) and I concatenate two arrays. It may be not the most elegant solution, but I can't see any array-to-string conversion (even implicitly) here.
Please advise

ui/BoxHighlightAppIconHoverOptions.ui Show resolved Hide resolved
@Hirnmoder
Copy link
Author

Hey. I've tried to use your PR locally and found some bugs. Some of them are commented here in review.

Also border radius is not working for hovering and active icons. I'm not sure why.

Hey Arilas,
thank you for testing my PR :)

The border radius with active icon bug sounds suspicious. May I request your software stack? (so on which distribution of Linux did you test it and what was the version of GNOME?). I am still on Fedora 40 with GNOME 46 and no support for CSS variables. It is possible that my workaround-code for CSS variables works (GNOME <= 46), but my code for the actual implementation (GNOME >= 47) does not. I'll have to look into this.

For the other bugs in question, I'll leave a response directly at your code remarks.

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