Switch matching profiles #389
Conversation
ext/bg/js/backend.js
Outdated
| if (!contextHasResults) { | ||
| yield {index: profileCurrent, profile: profiles[profileCurrent]}; | ||
| } |
There was a problem hiding this comment.
Could be useful to yield all profiles but also indicate which is the currently selected profile in case the user has not specified any conditions or none of them match the current context
There was a problem hiding this comment.
Fixed in eacae54
siikamiika
force-pushed
the
switch-matching-profiles
branch
from
5d93d0d to
eacae54
Compare
|
There are potentially some issues preparing |
This comment has been minimized.
This comment has been minimized.
siikamiika
force-pushed
the
switch-matching-profiles
branch
from
f6b5cb5 to
a4df89c
Compare
|
7b7db89 could have actually been done on master first and rebased on this, but it doesn't change anything in practice except one property access so let's include it here while we're at it. |
|
So you're right, this is something that will need a UI/UX pass before fully implementing, since there have been issues before with users disliking some UI layout settings (#349). I have created #402 to have a separate discussion on UI design changes. Changing the existing UI design should be considered outside the scope of this PR. |
siikamiika
force-pushed
the
switch-matching-profiles
branch
from
7b7db89 to
196d7a6
Compare
|
Ok, I think I'll go with the current design and the profile name to the floating header, making it only visible when there are more profiles than one. After that I'll add some hotkeys to switch between them. |
|
It should now be usable with a reasonable amount of active profiles without changing the current layout too much. |
|
Sorry for going back and forth with the commits, I can try to squash related ones at some point |
siikamiika
force-pushed
the
switch-matching-profiles
branch
from
2294726 to
45f6ea4
Compare
| getOptionsContext() { | ||
| throw new Error('Override me'); | ||
| return { | ||
| index: this.profileSwitcher.globalProfileIndex |
There was a problem hiding this comment.
This could be safer if instead of the index a unique ID was used. It could be either autoincrement from the DB or a randomly generated number. Not sure if same applies to backend/options because the index comes from there.
ext/bg/js/search.js
Outdated
| onProfileChanged(profileIndex) { | ||
| super.onProfileChanged(profileIndex); | ||
| const query = this.query.value; | ||
| if (query) { | ||
| this.setQuery(query); | ||
| this.onSearchQueryUpdated(query, false); | ||
| } | ||
| } |
There was a problem hiding this comment.
This will be called from prepare before QueryParser has been prepared. It throws an error about missing template handler. Will have to look for a workaround, for example using a prepared flag.
siikamiika
force-pushed
the
switch-matching-profiles
branch
from
45f6ea4 to
7d022a7
Compare
The first few commits are refactoring and framework building, and c7d0df4 is the actual feature.
The pattern used in f59e760 helps with #368. It took me a while to completely (maybe) undestand how the popup proxy framework works, and did some misguided work on siikamiika@875b04b. While most of it is useless, I think the "selective flooding algorithm" can be reused for iframes #29 #364.
I'd appreciate some ideas about displaying the dropdown menu or profile selection. It can be quite ugly depending on the operating system and theme.
Fixes #388
Possible workaround for #320, #378, #379