Fixed interaction with custom shell themes #271

jambonmcyeah · 2020-05-04T03:31:59Z

Combines my previous PR #269 with @scaryrawr 's PR #270

The reason for this is addressed in the comments on the comments in #270

Closes #231, Closes #230

isantop · 2020-05-05T14:06:39Z

src/search.ts

@@ -29,7 +29,7 @@ export class Search {
    ) {
        this.select_cb = select;
        this.dialog = new ModalDialog({
-            styleClass: "pop-shell-search",
+            styleClass: "pop-shell-search modal-dialog",


I like that we're using existing classes where they make sense here.

isantop · 2020-05-05T14:07:08Z

src/search.ts

@@ -170,13 +182,13 @@ export class Search {

                let label = new St.Label({
                    text: title,
-                    styleClass: "pop-shell-search-label",
+                    styleClass: "pop-shell-search-label list-search-result-title",


Here (and the next one) also show good use of existing, supported style classes.

src/extension.ts

isantop · 2020-05-05T14:13:59Z

Pushed a copy of this branch into our repo for building/CI purposes. Please merge this PR instead of that one.

src/search.ts

jambonmcyeah · 2020-05-05T18:36:29Z

@isantop Completed all your requests

isantop · 2020-05-05T18:55:23Z

src/search.ts

                    y_align: Clutter.ActorAlign.CENTER
                });

                label.clutter_text.set_ellipsize(Pango.EllipsizeMode.END);

-                let container = new widgets.Box({ styleClass: "pop-shell-search-element" })
+                let container = new widgets.Box({ styleClass: "pop-shell-search-element list-search-result" })


This looks better in Pop and Adwaita, but in many themes it causes the text to become unreadable. I think the existing styling present for these widgets was fairly theme-agnostic and resulted in guaranteed-readable text.

jambonmcyeah · 2020-05-05T21:36:26Z

I addressed all your requests. While fixing the issues I noticed a bug with stylesheets only loading when the extension is initially loaded and fixed it. Also added detection for shell theme changes instead of relying on gtk theme changes, since they can be changed separately.

jambonmcyeah · 2020-05-05T21:42:40Z

There's an issue with using custom styling, which is themes that style the shell widgets dark even on light variants (Adwaita, Adapta, Plata, Ant-Dracula, etc.) don't work well. I removed the color in the css to make them work better, however it still doesn't work great. This is why I originally set the class of the search elements to button to get around this issue.

When using `loadTheme` it merges the default GNOME theme with the style sheet passed into `setThemeStylesheet`. This change instead uses the existing theme and loads the pop style sheet on top of it (if a previous theme exists). Co-authored-by: Jun Bo Bi <jambonmcyeah@gmail.com>

jambonmcyeah · 2020-05-06T05:00:29Z

The real solution to this problem is to implement search items using St.Button which is the implementation in gnome-shell, instead of detecting arrow keys manually and faking focus. A proper implementation would enable mouse support and be compatible with custom themes. Yet I don't have nearly enough expertise on gtk and gnome to implement this properly, so I guess we have to live with this imperfect solution for now.

isantop · 2020-05-06T15:17:59Z

@mmstick Some notes about the implementation of the Launcher we might consider ☝️

isantop · 2020-05-06T15:19:52Z

@jambonmcyeah Can you post a screenshot of the issues you are seeing with the stock styling? I worked pretty hard to try and get the styling to be fairly theme-neutral while also looking good.

isantop

Using a dark theme, the current revision results in very poor contrast for the currently selected item. This affects Pop-Dark, Arc-Dark, Adwaita, Yaru-Dark, Materia-Dark, and Adapta-Nokto. There are likely others as well, but those are the ones I have handy for testing.

jambonmcyeah · 2020-05-06T16:01:10Z

Should be fixed now, I tested all the themes you mentioned and some more and they all work fine
The original issue was that some themes style shell widgets dark even on light variants, I hard-coded most of those themes and it's fixed.

- Launcher uses shell theme, not application theme - Checking shell enables dark application theme with light shell theme Co-authored-by: Jun Bo Bi <jambonmcyeah@gmail.com>

isantop · 2020-05-07T15:14:36Z

@pop-os/quality-assurance Testing branch for this is now jambonmcyeah-shell-theme-fix

isantop

Looks fine from my end.

mmstick

I'll do some followup improvements to the code from here on. Thanks!

Issue: Pop Shell reverts the shell theme to the default shell theme on startup. New Behavior: Allow using custom themes with Pop Shell. Stop reverting custom themes back to the default theme. Fix all interactions with dark and light themes revolving around custom themes. Origin: #271 Closes #230, Closes #231

nicolae-stroncea · 2020-05-07T23:53:06Z

This also fixes #227 and #216

Issue: Pop Shell reverts the shell theme to the default shell theme on startup. New Behavior: Allow using custom themes with Pop Shell. Stop reverting custom themes back to the default theme. Fix all interactions with dark and light themes revolving around custom themes. Origin: #271 Closes #230, Closes #231

This was referenced May 4, 2020

Fixed interaction with custom shell themes #269

Closed

Load pop stylesheet into existing user theme #270

Closed

Pop Shell does not respect shell theme choice #231

Closed

jambonmcyeah force-pushed the shell-theme-fix branch 2 times, most recently from cf61067 to 28607a6 Compare May 5, 2020 01:59

isantop requested review from isantop and a team May 5, 2020 14:05

isantop approved these changes May 5, 2020

View reviewed changes

isantop self-requested a review May 5, 2020 14:15

isantop suggested changes May 5, 2020

View reviewed changes

src/search.ts Outdated Show resolved Hide resolved

jambonmcyeah force-pushed the shell-theme-fix branch from 28607a6 to 1fd33c8 Compare May 5, 2020 18:34

jambonmcyeah requested review from isantop and removed request for a team May 5, 2020 18:41

isantop requested a review from a team May 5, 2020 18:43

isantop suggested changes May 5, 2020

View reviewed changes

jambonmcyeah force-pushed the shell-theme-fix branch 2 times, most recently from ab56749 to a232e6d Compare May 5, 2020 21:33

jambonmcyeah force-pushed the shell-theme-fix branch from a232e6d to bd65784 Compare May 5, 2020 21:39

jambonmcyeah requested review from isantop and removed request for a team May 5, 2020 21:43

scaryrawr and others added 2 commits May 5, 2020 17:53

Fix search interaction with custom shell theme

d6d3c08

jambonmcyeah force-pushed the shell-theme-fix branch from bd65784 to b202a13 Compare May 5, 2020 21:55

isantop suggested changes May 6, 2020

View reviewed changes

jambonmcyeah force-pushed the shell-theme-fix branch 3 times, most recently from b6fab69 to fa89e55 Compare May 6, 2020 16:00

jambonmcyeah requested a review from isantop May 6, 2020 16:01

jambonmcyeah force-pushed the shell-theme-fix branch 2 times, most recently from b815ab0 to 7854812 Compare May 6, 2020 16:05

Load pop light/dark style based on shell theme

b232b1a

- Launcher uses shell theme, not application theme - Checking shell enables dark application theme with light shell theme Co-authored-by: Jun Bo Bi <jambonmcyeah@gmail.com>

jambonmcyeah force-pushed the shell-theme-fix branch from 7854812 to b232b1a Compare May 6, 2020 16:10

mmstick changed the base branch from master_focal to dev_focal May 6, 2020 17:16

mmstick force-pushed the dev_focal branch 2 times, most recently from c372d02 to 9d4271f Compare May 6, 2020 17:27

isantop requested a review from a team May 7, 2020 15:14

isantop approved these changes May 7, 2020

View reviewed changes

mmstick mentioned this pull request May 7, 2020

[20.04] Custom shell theme won't stick pop-os/beta#99

Closed

mmstick approved these changes May 7, 2020

View reviewed changes

mmstick merged commit 623a10f into pop-os:dev_focal May 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed interaction with custom shell themes #271

Fixed interaction with custom shell themes #271

jambonmcyeah commented May 4, 2020 •

edited

Loading

isantop May 5, 2020

isantop May 5, 2020

isantop commented May 5, 2020

jambonmcyeah commented May 5, 2020 •

edited

Loading

isantop May 5, 2020

jambonmcyeah commented May 5, 2020 •

edited

Loading

jambonmcyeah commented May 5, 2020 •

edited

Loading

jambonmcyeah commented May 6, 2020 •

edited

Loading

isantop commented May 6, 2020

isantop commented May 6, 2020

isantop left a comment

jambonmcyeah commented May 6, 2020 •

edited

Loading

isantop commented May 7, 2020

isantop left a comment

mmstick left a comment

nicolae-stroncea commented May 7, 2020

Fixed interaction with custom shell themes #271

Fixed interaction with custom shell themes #271

Conversation

jambonmcyeah commented May 4, 2020 • edited Loading

isantop May 5, 2020

Choose a reason for hiding this comment

isantop May 5, 2020

Choose a reason for hiding this comment

isantop commented May 5, 2020

jambonmcyeah commented May 5, 2020 • edited Loading

isantop May 5, 2020

Choose a reason for hiding this comment

jambonmcyeah commented May 5, 2020 • edited Loading

jambonmcyeah commented May 5, 2020 • edited Loading

jambonmcyeah commented May 6, 2020 • edited Loading

isantop commented May 6, 2020

isantop commented May 6, 2020

isantop left a comment

Choose a reason for hiding this comment

jambonmcyeah commented May 6, 2020 • edited Loading

isantop commented May 7, 2020

isantop left a comment

Choose a reason for hiding this comment

mmstick left a comment

Choose a reason for hiding this comment

nicolae-stroncea commented May 7, 2020

jambonmcyeah commented May 4, 2020 •

edited

Loading

jambonmcyeah commented May 5, 2020 •

edited

Loading

jambonmcyeah commented May 5, 2020 •

edited

Loading

jambonmcyeah commented May 5, 2020 •

edited

Loading

jambonmcyeah commented May 6, 2020 •

edited

Loading

jambonmcyeah commented May 6, 2020 •

edited

Loading