jnsh / arc-theme

A flat theme with transparent elements (actively maintained fork)
GNU General Public License v3.0
902 stars 77 forks source link

[WIP] Adding gnome-shell 40 support #120

Closed zeten30 closed 3 years ago

zeten30 commented 3 years ago

Gnome-shell 40 initial support.

jnsh commented 3 years ago

Thanks for the pull request.

Since this is still marked WIP, can you clarify are you planning on completing the gnome-shell 40 port completely, or only adding the initial support and fixing some of the most obnoxious problems? I do appreciate the help in either case :)

However, for future maintenance, and in order for me to review the changes, any updates to the base 3.38 theme should be added in individual commits. Also, the commit messages should include links to the corresponding upstream commits, or otherwise explain the purpose of the change. You can have a look at the 3.38 theme commit history for an example.

If you're planning on finishing the port, you should look at the upstream commit log for the default shell theme, starting from the date they split the stable gnome-3-38 branch (usually this is done with the first stable point release, in this case 3.38.1 was released Oct 5th 2020). Then go trough every commit, and port the changes to Arc if necessary.

With the current PR, I can't differentiate the changes to the actual theme from previous version, so I can't comment on those. The meson.build update however looks exactly what I had in mind myself, so I'd be happy with merging that :)

Sorry about the long reply, and I hope none of this was discouraging. Let me know if any of this was unclear, or if you need any further advise.

zeten30 commented 3 years ago

@jnsh Hi, no problems with the long reply. I've expected it and that's why I've marked it as WIP.

I'm glad that you like the meson.build stuff. I'm also fine with completing gnome-shell 40 port completely or as a collaboration (if your fine with helping me with it).

If I understand it correctly we want to break it into smaller commits, correct? Something like:

  1. add support to meson
  2. create a directory structure for version 40
  3. 4,5 change scss files..

Actual style changes are tiny.

$ diff 3.38.scss 40.scss
891c891
<       border-bottom-width: 1px;
---
>       border: 0 0 1px 0;
892a893,894
>       border-radius: $panel-corner-radius;
>       box-shadow: none;
898a901,902
>       border: 0 0 1px 0;
>       border-radius: $panel-corner-radius;
900d903
<       border-bottom-width: 1px;
1768a1772
>   background-image: none;
1797c1801,1804
<   &, &:rtl, .right & { padding: 4px 8px; }
---
>   &, &:rtl, .right & { 
>     padding: 4px 8px; 
>     border-radius: 0;
>   }
1799c1806,1814
<   .top &, .bottom & { padding: 6px; }
---
>   .top &, .bottom & { 
>     padding: 6px;
>     border-radius: 0; 
>   }
> }
> 
> .dash-background {
>   background-color: transparent;
>   border-radius: 0 
2132a2148
>   background-image: none;

It fixes weird shadows and rounded corners on panel buttons & dash. The only think I'm little struggling with is rounded corners of workspaces in overview.

jnsh commented 3 years ago

If I understand it correctly we want to break it into smaller commits, correct? Something like:

1. add support to meson

2. create a directory structure for version 40

3. 4,5 change scss files..

Pretty much so. Step 2 should also include copying over the shell theme files from previous version i.e. cp -a common/gnome-shell/3.38 common/gnome-shell/40, but I think this was already clear :)

For the changes to SCSS files and other theme files, it really is best to methodologically go trough the upstream changes starting from the oldest commits, to make sure nothing is missed, and to keep the Arc SCSS in sync with upstream. This also helps the review process and any collaborative working, when it's made clear which upstream changes have been addressed.

If there are multiple upstream commits effecting a single shell element (e.g. for dash), it might make sense to group the changes to a single commit for Arc. Also some changes may have already been backported to earlier versions, and therefore to Arc. Also there could be some changes that simply aren't necessary for Arc for some reason.

I have quite a lot of work I'd like to get finished ASAP for Arc, so I'm happy with any amount of help on this. Feel free to ask for any advice, or if anything gets too difficult, you can leave it to me. Again, the main thing is to be clear about which upstream changes have been addressed.

I don't see anything wrong with the SCSS diff you posted, but I can't be certain without looking up references to the upstream changes. Also I don't have GNOME 40 installation set up yet, so I can't actually test the changes at this time.

Also please have a look a the notes for commit messages at the end of HACKING.md

jnsh commented 3 years ago

So I finally managed to get GNOME 40 installed for testing...

I think the theme update will be quite a bit trickier this time than usual. This is not necessarily for technical reasons, but there are some design-related decisions and tweaks or fixes that aren't simple if you haven't worked on the theme before. Therefore it may be easier if I'll finish the porting myself this cycle, especially since I'd like to get the update out around the time of the stable GNOME 40 release, which is only 5 days away..

If you have already made a lot of progress ahead after the initial PR, I can review that, and continue the work myself from there. From the current PR, I could merge the meson.build update, if you clean up the commit from other changes, and update the commit message.

Sorry about the hassle. I greatly appreciate your effort in any case.

zeten30 commented 3 years ago

@jnsh understood:) So far, my focus was only on fixing 'rounded corners' and 'shadows' for panel buttons. Nothing fancy.

I've removed everything besides meson.build.

zeten30 commented 3 years ago

My changes. Just for reference. _common.zip

jnsh commented 3 years ago

I merged the meson.build changes as part of the initial GNOME 40 updates, although I made a few more cosmetic tweaks, and separated the addition to the gnome_shell_versions array from this commit.

Thanks again!