Skip to content
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.

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

Per style options #827

Closed
eight04 opened this issue Jan 24, 2020 · 25 comments · Fixed by #1358
Closed

Per style options #827

eight04 opened this issue Jan 24, 2020 · 25 comments · Fixed by #1358

Comments

@eight04
Copy link
Collaborator

eight04 commented Jan 24, 2020

I'd like to discuss

  1. What should be put in the per-style options page?
  2. How do we implement the per-style options page?

What should be put in the per-style options page?

How do we implement the per-style options page?

I'd like to avoid modifying manage.html to prevent conflicts with #636. I think we can put the per-style options in the usercss options dialog as a new tab (and maybe the default tab). Therefore, we will get a gear icon for each style consistently.

@narcolepticinsomniac
Copy link
Member

narcolepticinsomniac commented Feb 8, 2020

My bad, I didn't even notice this issue till I just searched "inclusion".

Therefore, we will get a gear icon for each style consistently.

I guess you're talking about in the manager? I don't see it working in the popup, particularly inclusions.

I think we can put the per-style options in the usercss options dialog as a new tab (and maybe the default tab)

Why a new tab though? Using the usercss dialog in the manager seems logical, but dialogs are not so intertwined with the rest of the UI, that conflicts aren't gonna be some major issue. I don't recall exactly, but I have to think Mottie had all the dialogs functioning properly in the redesign. For the most part, it should be as simple as incorporating the icons.

There's no real reason we need to avoid the manager, or wait for anything. I kinda thought you just wanted to take a break from it at the time. We could incorporate per-style options into either of the existing dialogs, and it should be pretty painless to transition them into the manager redesign, if and when it ever happens.

Edit: Rereading this, I'm thinking you meant a new tab inside the dialog. That didn't really register, since most of my styles aren't usercss, so there'd be no need. I think if you're worried about space, tabs inside a dialog is not the ideal solution. Why not keep them separate and use messageBox() instead?

@eight04
Copy link
Collaborator Author

eight04 commented Feb 9, 2020

Why not keep them separate and use messageBox() instead?

  1. Consistently display the gear icon.
  2. Avoid adding a new icon in the manager and the popup.
  3. Avoid modifying manage.html.

most of my styles aren't usercss, so there'd be no need

We can hide the tab bar if usercss options are unavailable.

@narcolepticinsomniac
Copy link
Member

narcolepticinsomniac commented Feb 9, 2020

Avoid modifying manage.html

I don't think that needs to be a major concern. That manager PR probably already needs quite a bit of work. Whatever changes we make should be pretty trivial to integrate.

I'm not against using usercss dialogs though. I didn't realize you wanted to incorporate per-style options in the popup as well. Inclusions will be a little odd, since only applicable styles are listed. Otherwise, it seems like you have have a good idea what you wanna incorporate, and how. It all sounds pretty solid.

@FranklinYu
Copy link

Just to make sure: is the “User inclusion/exclusion” item referring to a list of them? If so then I would like to vote for it. My use case is that I excluded Stack Overflow from “Stack Overflow Dark” style, but in future I would like to review this decision.

@narcolepticinsomniac
Copy link
Member

PSA

@eight04
Copy link
Collaborator Author

eight04 commented Feb 10, 2021

I have tried to extend usercss config dialog but then found it's too hard to work with the UI.

How about adding a Style settings button to the editor and open a dialog when clicked?

@tophf
Copy link
Member

tophf commented Feb 10, 2021

I think the invocation place is the trivial part. The important part is that the implementation should be self-contained so we could place it anywhere. I think it can fit in the usercss config as an additional column to the left of the usercss config if the window width is sufficient or at the top otherwise.

@narcolepticinsomniac
Copy link
Member

it's too hard to work with the UI

How so? I thought the general idea was give usercss configs to all entries in the popup and manager, and if the styles actually have usercss configs, make them two tabs inside the config dialog.

@eight04
Copy link
Collaborator Author

eight04 commented Feb 10, 2021

Some problems about the button bar: should 2 tabs (usercss var and style settings) share the same button bar? I think style settings should save on change by default so we don't need a button for them. Maybe we should also make usercss var save on change so we can get rid of button bar entirely.

@tophf
Copy link
Member

tophf commented Feb 10, 2021

Buttons can stay as is, I see no problem with using them for both types.

Regarding tabs, style settings can be always displayed in a column to the left, otherwise people won't find that additional tab. It can be narrow initially, expanded (or activated) on click:

image

@eight04
Copy link
Collaborator Author

eight04 commented Feb 10, 2021

What would reset button do for style settings?

@tophf
Copy link
Member

tophf commented Feb 10, 2021

It can be simply disabled :-) but that's not fun. I guess it can reset user-specified stuff which we would indicate in the same fashion by adding a small x next to the changed/customized item.

@yam-liu

This comment has been minimized.

@eight04
Copy link
Collaborator Author

eight04 commented Oct 20, 2021

I can start prototyping this feature after merging #736. However, I will build the UI in the editor which looks easier.

Then, decide if we should also add it to the dialog or introduce a build system first so it will be easier to modular HTML templates. (or just stay with the editor like other userscript managers)

@tophf
Copy link
Member

tophf commented Dec 17, 2021

However, I will build the UI in the editor which looks easier.

I've tried using it and I don't like it. Our editor already has a lot of stuff in the UI so the tab bar in a place that already has a lot of text is really hard to notice. While we could make it more prominent, it wouldn't change the weirdness and increased complexity of the layout, which previously was simple: a header on the left and the code on the right.

Combining style settings and style variables dialog seems to me the only sensible solution. It'll be easily discoverable because a config dialog is the obvious choice, especially in the popup. We can make the dialog look good and self-explanatory by experimenting with its layout (e.g. the above example or something entirely different). We can also add an icon/button to open the config modal in the editor for convenience.

@eight04
Copy link
Collaborator Author

eight04 commented Dec 18, 2021

Combining style settings and style variables dialog seems to me the only sensible solution. It'll be easily discoverable because a config dialog is the obvious choice, especially in the popup.

It's convenience to configure style variables in the popup, because you can see the change without leaving the page.

For other styles settings (e.g. update URL?) that doesn't benefit from popup, I think it is more usable in the style manager or editor:

  1. The popup only shows styles that are applied to the page.
  2. The popup closes when losing focus.
  3. The popup has a narrower width.

@tophf
Copy link
Member

tophf commented Dec 18, 2021

By convenience I meant the user's workflow. Most users don't know our UI at all. They install a style, they open a site, they want to tweak something in the style, they click the extension icon, see their style, they click the config button, and they see everything that can be configured there.

Do you think they will guess there's a separate config hidden in an editor under a small tab? No. They don't want to edit a style so they'll never find this UI naturally.

@tophf
Copy link
Member

tophf commented Dec 18, 2021

As for styles that aren't shown in the popup, the users will find them in the style manager, and press the config button there.

The popup closes when losing focus.

Not a problem. This is how all extensions behave so the users will learn not to close the popup.
We can also autosave the current values in a temporary storage and prompt to restore them next time the popup is shown.

The popup has a narrower width.

We can resize it temporarily when the user focuses a textarea or allow the user to resize the popup or we may add a button to open the config in a new small window.

@eight04
Copy link
Collaborator Author

eight04 commented Dec 18, 2021

Do you think they will guess there's a separate config hidden in an editor under a small tab?

We don't collect user data so I (probably we?) don't know. Some userscript managers e.g. Tampermonkey/Violentmonkey put per-script settings in the editor so maybe we can ask them if we want.

@tophf
Copy link
Member

tophf commented Dec 18, 2021

They don't have an existing panel with tons of settings like we do. Adding a tab bar breaks the concept, so its discoverability is unavoidably worse. They also don't have a config dialog like we do, which is a place where any user would expect the settings to be. I don't see a single redeeming point in the current UI.

@tophf
Copy link
Member

tophf commented Dec 18, 2021

Another problem with the settings tab in the editor is that it wastes horizontal space on a typical desktop screen when it's shown while also wasting ~2 rows of code vertically, which is pretty precious on notebooks, when not shown.

To reiterate, I'm ardently against the current UI.

In the future, we can find/write a js library that implements panels: dockable, draggable, collapsible (so the panel is revealed when its title is clicked), mergeable. Ideally, just the way it's done in Visual Studio so the user can arrange the workspace flexibly. We'll use them for editor settings, style settings, style variables, linter, sections, publish, and any other new things.

@tophf
Copy link
Member

tophf commented Dec 19, 2021

For simplicity, the new UI should use a separate modal, not yet combined with the config dialog. I've already suggested it in the PR, along with removing the Back to manage button, so here's a screenshot to illustrate the idea:

image

The modal will be identical to lint config. The extra files will be loaded after the button is clicked.

@tophf
Copy link
Member

tophf commented Dec 20, 2021

Ah, currently the changes are remembered immediately, but in the modal I think we should display Ok/Apply/Cancel/Undo buttons and toggle their disabled status when the info changes. For simplicity, we won't use DirtyReporter, instead the input listener will compare all current values to the old values and toggle the buttons accordingly. It should also set some kind of editor.isSettingsDirty boolean so that our global beforeunload handler can prevent the tab from closing when there are unsaved changes. The Undo button - if we implement it - will be enabled after Ok/Apply is clicked, and will restore the initial saved/default values.

@eight04
Copy link
Collaborator Author

eight04 commented Dec 21, 2021

In dialog, I think OK/Cancel button works nicely with a simple input box e.g. lint config, hotkey chooser, etc, which is a fill-and-submit flow.

When the page is a setting page composed with multiple controls, it is more comfortable for users to save automatically on change like configuration UI in modern apps, which is a change-and-reflect flow.

Reset button is still useful for resetting the setting to default. Not sure about undo button.

@tophf
Copy link
Member

tophf commented Dec 21, 2021

Hmm, maybe. Then a single Close button and a short label saying that the changes are saved automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants