Spike: refactor disable/enable logic for statusbar widgets via composition #8240
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
4 times, most recently
from
47aa461 to
78c451b
Compare
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
7 times, most recently
from
5a4254d to
8ceec13
Compare
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
6 times, most recently
from
3f6c414 to
711cd44
Compare
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
6 times, most recently
from
a4c6854 to
c65b057
Compare
I am still not very happy with the outcome here. We need to keep `urltype` on the widget class, because it needs to be exposed to as a `pyqtProperty` to make it available to the stylesheet.
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
3 times, most recently
from
437a234 to
76e2ad6
Compare
Moving the factory out of `StatusBar`, as the only `self` used in there is being passed as `parent` to `ProgressWidget`. Heavily influenced by the idea @toofar explained in his comment[1]. It's not a decorator, yet, but after thinking for too long about this approach I needed some tangible feedback by building something. I am still not quite sure what to use as a unique key for each `StatusBarItem`, as we need to look them up from other places (e.g. `MainWindow`[2]) eventually. [1] qutebrowser#7407 (comment) [2] https://github.com/pylbrecht/qutebrowser/blob/d1c7f9482662d1aacd252a7876ff501f3c7b18af/qutebrowser/mainwindow/mainwindow.py#L516-L546
One more place, where we are now able to remove direct references to `StatusBarItem`s.
Looks like `_text_widgets` is on longer necessary. I hope this is the result of my glorious refactoring skills and not just me being too tired to realize I am missing something.
pylbrecht
force-pushed
the
statusbar-composition-spike
branch
from
76e2ad6 to
6194126
Compare
Probably a better place to store these globally.
Apparently we used the same widget instance for both scroll and scroll_raw. Not sure if that was intentional.
Nothing is referencing this, so we can safely remove it.
I am not too happy about that, because not all subclasses will implement `on_tab_changed()`. I hope I will find another way to iron this out later. For now it's at least something.
toofar
reviewed
Sep 14, 2024
| objects.statusbar_items["scroll"].on_tab_changed(tab) | ||
| objects.statusbar_items["scroll_raw"].on_tab_changed(tab) | ||
| objects.statusbar_items["history"].on_tab_changed(tab) | ||
| for item in objects.statusbar_items.values(): |
There was a problem hiding this comment.
Regarding the commit message:
I am not too happy about that, because not all subclasses will implement
on_tab_changed()
The options I see are a) this, make signals part of the StatusBarItem interface and make them optional to implement, b) have subclass items connect to signals they need on their own (either from the global object interfaces or from the MainWindow interface which could be passed down in the constructor).
There might be other options though, I haven't thought about it too much.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are WIP changes. I am using this PR to track progress.
TODO
backforward.pyclock.pykeystring.pypercentage.pyprogress.pysearchmatch.pytabindex.pyurl.pytextbase.pyStatusBarIteminterfaceStatusBarIteminterface inStatusBar(final integration step)StatusBarItemsubclassesbackforward.pyclock.py(tricky)keystring.pypercentage.pyprogress.pysearchmatch.pytabindex.pyurl.py(tricky;pyqtProperty)textbase.pyBackforwardinstead ofBackforwardWidget)Open questions
Commandwidget? Seems special compared to the ones above.Relates to #7407