Skip to content

ongoingFactEntry: Remove custom style class #379

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.

Sign up for GitHub

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

Open
slovdahl wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

ongoingFactEntry: Remove custom style class #379

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions extension/widgets/ongoingFactEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class OngoingFactEntry extends St.Entry {
name: 'searchEntry',
can_focus: true,
track_hover: true,
hint_text: _("Enter activity…"),
style_class: "search-entry"
hint_text: _("Enter activity…")
Copy link
Author

@slovdahl slovdahl Aug 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have no idea if this is the right way to fix it, but opening a PR just to start the discussion. The custom style was added in e7b2e5c.

Copy link
Contributor

@mwilck mwilck Sep 1, 2025

Choose a reason for hiding this comment

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

I don't see this issue. It might be related to your gtk theme settings.

The commit that introduced it is ancient.

Copy link
Author

@slovdahl slovdahl Sep 1, 2025

Choose a reason for hiding this comment

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

Thanks for the hint! I was thinking I have a fairly standard Ubuntu 24.04 with GNOME.. But I installed a clean Ubuntu 24.04 in a VM and was unable to reproduce the same problem.

After searching through my home directory I found a reference to search-entry in the Blur my Shell extension. And indeed, after installing Blur my Shell in the VM I immediately got the same problem.

So maybe I should report the issue there instead?

Copy link
Author

@slovdahl slovdahl Sep 1, 2025

Choose a reason for hiding this comment

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

Looking back at #103 it seems like it was a workaround added for old GNOME versions 🤔 Or? So is it needed for anything any more?

Copy link
Contributor

@mwilck mwilck Sep 2, 2025

Choose a reason for hiding this comment

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

Honestly, I don't know. I'm not sure if current GNOME versions still use style_class at all (but I'm assuming it does, otherwise removing wouldn't have an effect, right?).

In general I would say it is innocent. We're not mandating anything about background or font colors or any other specific style attributes. It's just an abstract hint about the purpose of the widget. AFAICT this is how it should be done. It's then up the theme to determine how `search-entry´ widgets should be rendered.

I haven't heard about "blur my shell" so far. I'm generally sceptical about extensions that only add eye candy. As the authors of that extension seem to care a lot about this kind of things, maybe they'll be able to fix it, or at least provide some insight about style_class and how they handle it.

Copy link
Contributor

@mwilck mwilck Sep 2, 2025

Choose a reason for hiding this comment

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

I am a bit confused why an extension like "blur my shell" is necessary at all. To my understanding, this is what themes are for. Possibly the extension is doing stuff that can't be done with themes. But then, effects like the one you describe should be expected. The extension would need to take utmost care to play well with the settings the current theme is applying, which (I suppose) will be hard.

});

this._controller = controller;
Expand Down