conversation-activity-filter - Add hotkey hint#9349
Conversation
b8c6ed8 to
349807b
Compare
|
I don't know how to vertically center text. Fix it yourself if you can 
|
Co-authored-by: fregante <me@fregante.com>
fregante
left a comment
There was a problem hiding this comment.
pineapple looking very very good
| applyState(state as State); | ||
| } | ||
|
|
||
| let currentState: State; |
There was a problem hiding this comment.
Now you can change the state without actually applying it
There was a problem hiding this comment.
What does this mean and why would you want that?
There was a problem hiding this comment.
Now you can change the state without actually applying it
This is a problem, and I don't see a way to keep the state in sync without introducing a global variable
There was a problem hiding this comment.
By the way we can use Svelte for this kind of advanced widgets as it would make it easier to deal with state—as long as GitHub doesn't already have "behaviors" registered via .js-* classes
There was a problem hiding this comment.
Can you elaborate please? I asked Copilot to wrap menu items with a Svelte component and the result was terrible: SunsetTechuila@4d97216
There was a problem hiding this comment.
Yes, that's pretty much it, but probably it could move the entire JSX to the .svelte file.
The advantage would be that the widget would be a self-contained UI control that you alter via attributes. I think however content scripts don't support custom elements so we can't quite use it the same way.
I've never explored it, but React/Svelte can help avoid procedural code like what we have here. I don't know if this is more complex/verbose and worth it. I just know keeping the DOM state up to date is a solved problem.
There was a problem hiding this comment.
Either way, the PR is ready for merge, it was just a suggestion/proposal for the future.
In your latest gif it looks good. The h in |
resolves #9331
fixes #9348
fixes #9350
Test URLs
this PR
Screenshot