Make the UI/UX of the CSS Filter tooltip better

ASSIGNED
Assigned to
(NeedInfo from)

Status

()

Firefox
Developer Tools: Inspector
P2
enhancement
ASSIGNED
2 years ago
2 years ago

People

(Reporter: pbro, Assigned: mahdi, NeedInfo)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [devtools-ux])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Bug 1055181 added a very handy tooltip for editing complex CSS Filter values in the rule-view. Instead of having to type each filter function by hand, this tooltip provides a more visual way of doing it.

It was however developed without any input from a UI/UX person and is therefore not the easiest to use.

Rachel Nabors presented it to some users and got interesting feedback:

- Folks often don't press the plus next to the drop down to add the filter. Adding the filter on selection would be ideal, but making the plus look more like a button might be a good stopgap.
- We all wish there was a slider interface for changing the levels from 0 to 100. Working with the numbers is too hard when you're just experimenting.
- The save filter option is super cool! But not obvious. A more familiar save icon on even the words "save preset" would be awesome.

There is in fact a slider, if you drag on the label, but it looks like it's not obvious enough if no one found out about it.

While we're at it, I think it would help if each filter had a specialized editor. Indeed, each filter function expects slightly different values. Some functions work with values between 0 and 1, others with values between 0 and 100, the shadow filter expects a totally different type of value, and the url filter expects a URL.
Making this more obvious in the UI would help too.
(Reporter)

Updated

2 years ago
Whiteboard: [devtools-ux]
(Assignee)

Updated

2 years ago
Assignee: nobody → mdibaiee
Here is my detailed input regarding the UI/UX:
- The tooltip should never produce invalid values. E.g. this is currently the case when you add a shadow() filter, though don't add a value.
- The buttons should have a tooltip explaining what they do.
- The slider for functions like blur() should be more fine-grained, i.e. it should change the value in 0.1 steps instead of 1.
- The spinner for functions like brightness() is too fine-grained, i.e. it should change the value in 1 steps instead of 0.1.
- When there are multiple filters that overflow the area of the tooltip, the scrollbar overlaps the delete buttons of the filters.
- Entering a preset name and hitting Enter, closes the tooltip instead of saving the preset.
- Saved presets should show the complete value in a tooltip in case it is too long to be shown completely.
- The button to delete a filter overlaps the unit when the preset list is displayed.
- The button to add a preset should be disabled as long as the 'Preset Name' field is empty.
- When adding the 'filter' property the icon for to open the tooltip is not shown. That means you first have to enter a valid value like 'none' before you get the icon. This is bad for people that don't know which values are available.
- When the 'filter' property has an invalid value, the icon to open the tooltip is not shown. This results in the same UX as above.

The latter two points are actually a general issue for all CSS related tooltips and are not related to the content of the tooltip. So please let me know if I should move them into a separate issue.

Sebastian
(Reporter)

Comment 2

2 years ago
Thanks Sebastian, all very good points.

(In reply to Sebastian Zartner [:sebo] from comment #1)
> - When adding the 'filter' property the icon for to open the tooltip is not
> shown. That means you first have to enter a valid value like 'none' before
> you get the icon. This is bad for people that don't know which values are
> available.
> - When the 'filter' property has an invalid value, the icon to open the
> tooltip is not shown. This results in the same UX as above.
> 
> The latter two points are actually a general issue for all CSS related
> tooltips and are not related to the content of the tooltip. So please let me
> know if I should move them into a separate issue.
Yes please do file a new bug for this. The way the filter icon was added was by doing the same mechanism as the color-picker icon, but really, it shouldn't be the same. The filter icon is for the whole property, whereas for colors, we parse the value to find color occurrences in it.

Comment 3

2 years ago
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0) 
> - The save filter option is super cool! But not obvious. A more familiar
> save icon on even the words "save preset" would be awesome.
I guess we could expand the sidebar by default (and make sure the sidebar state is saved when open/closed).

> There is in fact a slider, if you drag on the label, but it looks like it's
> not obvious enough if no one found out about it.
I like the current UI because it brings uniformity for all filters, you get a draggable label and a text input. The original iterations had a slider for values with limits, an number input for values without limits and a text input for the rest. The design didn't feel consistent. I agree the current slider UI should be made obvious though.

> While we're at it, I think it would help if each filter had a specialized
> editor. Indeed, each filter function expects slightly different values. Some
> functions work with values between 0 and 1, others with values between 0 and 100
> Making this more obvious in the UI would help too.
Values between 0 and 100 are the same as values from 0 to 1, the latter is in percent, the former has no unit. Although, the fact that the filter tooltip converts these values to percentages can be annoying.

(In reply to Sebastian Zartner [:sebo] from comment #1)
> Here is my detailed input regarding the UI/UX:
> - The tooltip should never produce invalid values. E.g. this is currently
> the case when you add a shadow() filter, though don't add a value.
True, maybe the invalid input could glow red.

> - The buttons should have a tooltip explaining what they do.
I think they do, they just don't show up because of a bug.

> - The slider for functions like blur() should be more fine-grained, i.e. it
> should change the value in 0.1 steps instead of 1.
> - The spinner for functions like brightness() is too fine-grained, i.e. it
> should change the value in 1 steps instead of 0.1.
You can tweak that by holding Shift/Alt/Ctrl (I don't remember well) while dragging.

> - Entering a preset name and hitting Enter, closes the tooltip instead of
> saving the preset.
Yes, this is very annoying.

> - The button to add a preset should be disabled as long as the 'Preset Name'
> field is empty.
True.
> - When adding the 'filter' property the icon for to open the tooltip is not
> shown. That means you first have to enter a valid value like 'none' before
> you get the icon. This is bad for people that don't know which values are
> available.
> - When the 'filter' property has an invalid value, the icon to open the
> tooltip is not shown. This results in the same UX as above.
Agreed,
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #2)
> (In reply to Sebastian Zartner [:sebo] from comment #1)
> > - When adding the 'filter' property the icon for to open the tooltip is not
> > shown. That means you first have to enter a valid value like 'none' before
> > you get the icon. This is bad for people that don't know which values are
> > available.
> > - When the 'filter' property has an invalid value, the icon to open the
> > tooltip is not shown. This results in the same UX as above.
> > 
> > The latter two points are actually a general issue for all CSS related
> > tooltips and are not related to the content of the tooltip. So please let me
> > know if I should move them into a separate issue.
> Yes please do file a new bug for this.

For reference, I filed this as bug 1217328.

(In reply to Tim Nguyen [:ntim] from comment #3)
> (In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0) 
> > - The save filter option is super cool! But not obvious. A more familiar
> > save icon on even the words "save preset" would be awesome.
> I guess we could expand the sidebar by default (and make sure the sidebar
> state is saved when open/closed).

I assume the presets are not used that often to show them by default. I'd prefer to just make it more obvious what the related button does.

> (In reply to Sebastian Zartner [:sebo] from comment #1)
> > Here is my detailed input regarding the UI/UX:
> > - The tooltip should never produce invalid values. E.g. this is currently
> > the case when you add a shadow() filter, though don't add a value.
> True, maybe the invalid input could glow red.

Would be ok for me. Another option would be to use an advanced widget to tweak the shadows, which always produces valid values. (Same for the url() filter.)

> > - The slider for functions like blur() should be more fine-grained, i.e. it
> > should change the value in 0.1 steps instead of 1.
> > - The spinner for functions like brightness() is too fine-grained, i.e. it
> > should change the value in 1 steps instead of 0.1.
> You can tweak that by holding Shift/Alt/Ctrl (I don't remember well) while
> dragging.

I realized that afterwards. Non-the-less I believe the behavior of the slider and the spinner should be consistent. So e.g. using the spinner or the slider for blur() should change the value by 0.1 and by 1 for brightness(). Currently the slider may change the values in 1 steps while the spinner changes them in 0.1 steps or vice versa.
I'm not sure if the Up/Down shortcuts should work accordingly (i.e. let Up/Down change the value for blur() by 0.1) or in consistency with generally tweaking numerical values in the CSS pane (i.e. always let Up/Down change the value by 1, even for blur()).

Sebastian
See Also: → bug 1217328
(Reporter)

Updated

2 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 5

2 years ago
(In reply to Sebastian Zartner [:sebo] from comment #4)
> (In reply to Tim Nguyen [:ntim] from comment #3)
> > (In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0) 
> > > - The save filter option is super cool! But not obvious. A more familiar
> > > save icon on even the words "save preset" would be awesome.
> > I guess we could expand the sidebar by default (and make sure the sidebar
> > state is saved when open/closed).
> 
> I assume the presets are not used that often to show them by default. I'd
> prefer to just make it more obvious what the related button does.
> 

Yeah, I agree that we have to make presets more obvious, I'm not sure about showing them by default,
probably we can do this for the first time, but I'm not sure about the UX of that.

> > (In reply to Sebastian Zartner [:sebo] from comment #1)
> > > Here is my detailed input regarding the UI/UX:
> > > - The tooltip should never produce invalid values. E.g. this is currently
> > > the case when you add a shadow() filter, though don't add a value.
> > True, maybe the invalid input could glow red.
> 
> Would be ok for me. Another option would be to use an advanced widget to
> tweak the shadows, which always produces valid values. (Same for the url()
> filter.)

That's a possiblity, but it brings too much complexity in my opinion, we can just default to a simple value.

> > > - The slider for functions like blur() should be more fine-grained, i.e. it
> > > should change the value in 0.1 steps instead of 1.
> > > - The spinner for functions like brightness() is too fine-grained, i.e. it
> > > should change the value in 1 steps instead of 0.1.
> > You can tweak that by holding Shift/Alt/Ctrl (I don't remember well) while
> > dragging.
> 
> I realized that afterwards. Non-the-less I believe the behavior of the
> slider and the spinner should be consistent. So e.g. using the spinner or
> the slider for blur() should change the value by 0.1 and by 1 for
> brightness(). Currently the slider may change the values in 1 steps while
> the spinner changes them in 0.1 steps or vice versa.
> I'm not sure if the Up/Down shortcuts should work accordingly (i.e. let
> Up/Down change the value for blur() by 0.1) or in consistency with generally
> tweaking numerical values in the CSS pane (i.e. always let Up/Down change
> the value by 1, even for blur()).

You can set the unit to 0.1 by holding the Alt key, both arrow keys and slider increase/decrease by 0.1; The same applies to shift, increasing/decreasing by 10. By changing behavior for different filter, we bring inconsistency and confusion, I don't like that idea.

But we must introduce these features to the user, probably by showing a simple message the first time users open the tooltip, introducing users to the tooltip and it's features such as presets, photoshop-style inputs, etc etc.
(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #5)
> (In reply to Sebastian Zartner [:sebo] from comment #4)
> > > > - The slider for functions like blur() should be more fine-grained, i.e. it
> > > > should change the value in 0.1 steps instead of 1.
> > > > - The spinner for functions like brightness() is too fine-grained, i.e. it
> > > > should change the value in 1 steps instead of 0.1.
> > > You can tweak that by holding Shift/Alt/Ctrl (I don't remember well) while
> > > dragging.
> > 
> > I realized that afterwards. Non-the-less I believe the behavior of the
> > slider and the spinner should be consistent. So e.g. using the spinner or
> > the slider for blur() should change the value by 0.1 and by 1 for
> > brightness(). Currently the slider may change the values in 1 steps while
> > the spinner changes them in 0.1 steps or vice versa.
> > I'm not sure if the Up/Down shortcuts should work accordingly (i.e. let
> > Up/Down change the value for blur() by 0.1) or in consistency with generally
> > tweaking numerical values in the CSS pane (i.e. always let Up/Down change
> > the value by 1, even for blur()).
> 
> You can set the unit to 0.1 by holding the Alt key, both arrow keys and
> slider increase/decrease by 0.1; The same applies to shift,
> increasing/decreasing by 10. By changing behavior for different filter, we
> bring inconsistency and confusion, I don't like that idea.

I know that and it's cool that Alt and Shift also work for the slider. Though what I said is that the current behavior between slider and spinner is already inconsistent. On all numeric functions the slider changes by 1, the spinner by 0.1.
It's not a big deal for me, though.

Sebastian
(Assignee)

Comment 7

2 years ago
(In reply to Sebastian Zartner [:sebo] from comment #6)

> I know that and it's cool that Alt and Shift also work for the slider.
> Though what I said is that the current behavior between slider and spinner
> is already inconsistent. On all numeric functions the slider changes by 1,
> the spinner by 0.1.
> It's not a big deal for me, though.
> 
> Sebastian

Oh, you're right. That's because if we want to be able to use 0.1 precision in the values, we must set the input's step value to 0.1, which makes the spinner change by 0.1. We can't make it consistent with slider and arrow keys, sadly.
(Assignee)

Comment 8

2 years ago
Any update on the status of this bug? Do we have a UX review yet?

Comment 9

2 years ago
(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #8)
> Any update on the status of this bug? Do we have a UX review yet?
Helen is working on it.
(Reporter)

Comment 10

2 years ago
Triaging (filter on CLIMBING SHOES).
Severity: normal → enhancement
Helen, I think this is waiting on UX from you, but no flags were ever set.

@mahdi, are you still interested in working on this bug?
Flags: needinfo?(mdibaiee)
Flags: needinfo?(hholmes)
Priority: -- → P2

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 12

2 years ago
I'm not sure if I'll be able to do this at the moment, semester exams are on the way and I have to deliver a project, too. I might be able to work on this (and just start contributing again) in summer.
Flags: needinfo?(mdibaiee)
(In reply to Gabriel Luong [:gl] (ΦωΦ) from comment #11)
> Helen, I think this is waiting on UX from you, but no flags were ever set.
> 
> @mahdi, are you still interested in working on this bug?

Still working on this, I've had other priorities. I'll attach spec this quarter.
Flags: needinfo?(hholmes)
(Reporter)

Updated

2 years ago
See Also: → bug 1277079
Created attachment 8758806 [details]
inline-editor-css-filters.png

Spec for what the CSS editor could look like as an inline editor based on Bug 1258390. There's also a demo of this (not fully functional, see the notes at the bottom) here: http://helenvholmes.com/devtools-ux/css-filter/
Attachment #8758806 - Flags: review?(pbrosset)
(Reporter)

Comment 15

2 years ago
Really like it. 

One question first: you can drag functions in the list to re-order them, but since there are 2 columns, does that mean items in the 2nd column come after items in the 1st? I think yes, but just checking it's not just going to be a top to bottom drag/drop, but the list wraps to other columns.

And one comment: removing url and drop-shadow does simplify things and helps with having a simple interface people are going to want to play with, *but* how will we deal with filters that do use these functions? Like, say you have a page with filter: drop-shadow(0 0 1px red); and then you open the editor? Because, obviously, the drop-shadow isn't going to be in the editor. Should it get removed from the filter value then? Or just remain there and then we show a warning saying that it's not displayed? But then it makes it hard to play with functions in the editor, and re-order them. Where should the drop-shadow be in the list?
Maybe we should add it to the list of functions in the editor but make it a text field, or just uneditable text, but at least so it's there.

Same question if someone enters several times the same function, which is valid in CSS. Then we'd have to show it several times in the editor somehow.

I agree with what you said in bug 1277182 comment 2, we want a filter editor that encourages people to play, that's the whole goal of it, so I'm totally on board for simplifying it (having sliders even if there is no bound to a value for example), but we do need a way to support existing values that may not match what the editor can display.
(Reporter)

Comment 16

2 years ago
Comment on attachment 8758806 [details]
inline-editor-css-filters.png

Clearing the review flag while we're having the discussion.
Attachment #8758806 - Flags: review?(pbrosset)
(Reporter)

Updated

2 years ago
Blocks: 1258390
No longer depends on: 1258390
(In reply to Patrick Brosset <:pbro> from comment #15)
> I agree with what you said in bug 1277182 comment 2, we want a filter editor
> that encourages people to play, that's the whole goal of it, so I'm totally
> on board for simplifying it (having sliders even if there is no bound to a
> value for example), but we do need a way to support existing values that may
> not match what the editor can display.

I agree that simplifying the editor to encourage playing with the values is good, though as Patrick mentioned it must still provide proper UI for the other cases to give a good UX.

> And one comment: removing url and drop-shadow does simplify things and helps
> with having a simple interface people are going to want to play with, *but*
> how will we deal with filters that do use these functions?

I believe they should be added back into the editor as text fields resp. drop-shadow() maybe even as multiple fields. This doesn't fit to the numeric values UI, but those two are surely often used as well and people should be able to play with them, too.

> Same question if someone enters several times the same function, which is
> valid in CSS. Then we'd have to show it several times in the editor somehow.

Having several times the same function should be displayed several times within the editor.

Numeric values extending the sliders' ranges also need to be handled. One solution may be to extend the slider range accordingly. So, for example, the brightness slider would have a range of 0% - 100% by default but when it has a value of 140% it extends to 140% (or even 200%).
Also, there should not be null values for the functions but rather should they get their default values assigned even when it's not a zero value. I.e. opacity() would have 100% by default. If the default value is assigned or the slider dragged to it, the function should be grayed out in the editor and removed from the returned CSS value like it's done for blur() or saturate().

Sebastian
And the sliders should all have the same length. The current UI is pretty inhomogeneous.

Sebastian
Created attachment 8760374 [details]
inline-editor-css-filters.png

(In reply to Patrick Brosset <:pbro> from comment #15)
> Really like it. 
> 
> One question first: you can drag functions in the list to re-order them, but
> since there are 2 columns, does that mean items in the 2nd column come after
> items in the 1st? I think yes, but just checking it's not just going to be a
> top to bottom drag/drop, but the list wraps to other columns.
I've added numbers to make this more clear—I think that will work.

> And one comment: removing url and drop-shadow does simplify things and helps
> with having a simple interface people are going to want to play with, *but*
> how will we deal with filters that do use these functions? Like, say you
> have a page with filter: drop-shadow(0 0 1px red); and then you open the
> editor? Because, obviously, the drop-shadow isn't going to be in the editor.
> Should it get removed from the filter value then? Or just remain there and
> then we show a warning saying that it's not displayed? But then it makes it
> hard to play with functions in the editor, and re-order them. Where should
> the drop-shadow be in the list?
> Maybe we should add it to the list of functions in the editor but make it a
> text field, or just uneditable text, but at least so it's there.
> 
> Same question if someone enters several times the same function, which is
> valid in CSS. Then we'd have to show it several times in the editor somehow.
I tried handling both of these in the same way: when there's a property that we've decided to not add a UI for, it will still appear in the editor in the order that it's been authored, but it will only be draggable. This means that we'll have to add in rows into the column for repeat elements and such.
Attachment #8758806 - Attachment is obsolete: true
Attachment #8760374 - Flags: review?(pbrosset)
(Reporter)

Comment 20

2 years ago
Comment on attachment 8760374 [details]
inline-editor-css-filters.png

I think that would work.
One last comment about default values and removing values:

Having no contrast defined in a filter isn't the same as having contrast(0). So we can't display  greyed out line with contrast(0) as you've done for invert(0) for example.
This, I guess, is why you chose to have contrast(null) instead.

As soon as you enter a value or drag the slider, I guess this enables the contrast function and adds it to the filter.
If you then want to get rid of it, you can't just enter 0 because that is not the default value.
Should you write null in the field instead?

How about just deleting the value in the field altogether? So defaults values would, in fact, be functionName().

1. blur(30px)    -----o---------
2. brightness()  o--------------
3. contrast()    o--------------
etc.
Attachment #8760374 - Flags: review?(pbrosset) → review+
(In reply to Patrick Brosset <:pbro> from comment #20)
> Comment on attachment 8760374 [details]
> inline-editor-css-filters.png
> How about just deleting the value in the field altogether? So defaults
> values would, in fact, be functionName().
> 
> 1. blur(30px)    -----o---------
> 2. brightness()  o--------------
> 3. contrast()    o--------------
> etc.

Or we could reset the value to the default when the user deletes the value and moves the focus away from it.
I.e. when the value is deleted from contrast(), pressing Tab or clicking somewhere else sets the value to 100% and the slider accordingly and grays out the item.

Sebastian
Created attachment 8761253 [details]
css-filter-spec-sheet.png

(In reply to Patrick Brosset <:pbro> from comment #20)
> Comment on attachment 8760374 [details]
> Having no contrast defined in a filter isn't the same as having contrast(0).
> So we can't display  greyed out line with contrast(0) as you've done for
> invert(0) for example.
> This, I guess, is why you chose to have contrast(null) instead.
So I edited the mockup in the "dealing with zeros" section to use your suggestions below since I think handles this a little better. In essence, by dragging the slider all the way to the left, you remove the value entirely. By dragging it to the right at all, you start with its "zero" property and add it to the list of available props. (So that it's possible for you to have an "opacity(0)" included in your list of filters.)
Attachment #8760374 - Attachment is obsolete: true
Attachment #8761253 - Flags: review?(pbrosset)

Comment 23

2 years ago
Comment on attachment 8761253 [details]
css-filter-spec-sheet.png

I like the new design, however it removes the following capibilities:
- Adding multiple instances of the same filter
- Removing filters
- Editing drop-shadows and urls, why can't we have `drop-shadow(_)` for now ?

A few comments:
- The numbers are not obvious as drag handles
- The filters that are not set should just be removed, since the proposed design forces an default arbitrary order for the filters. Also, how would the default values for brightness would be handled ? brightness's default value is 100%, not 0%.
(Reporter)

Comment 24

2 years ago
(In reply to Tim Nguyen :ntim from comment #23)
> I like the new design, however it removes the following capibilities:
> - Adding multiple instances of the same filter
> - Removing filters

I think the general idea is the following one: we mostly want this to be a way for people to discover css filters and easily play with them, without being scared away by having to type in complex css syntax. Sliders are inviting and playful. So as a user, you'd enter "filter" in the name field, and then would immediately be able to play around with the sliders to come up with some effects.
Now, if you already have a filter defined in a rule, one that has multiple times the same function, or functions in a different order, or whatever, then opening the editor should respect that.
The editor just won't allow you to duplicate or remove functions, but if you do open it from an existing filter value, then the corresponding functions will be shown.
If you're at the stage where you need to add several times the same function, then you probably know what you're doing and then the editor is probably not as helpful for you (again, sliders are nice to discover the feature, but not a great tool to choose specific values). At least that's what I understand of the reasoning.

It's nice because it allows us to define a much simpler and inviting UI than the one we have today.

> - Editing drop-shadows and urls, why can't we have `drop-shadow(_)` for now ?

I guess we could simply make the input field between the parenthesis accept any value, and therefore drop-shadow and url could be editable too.

> A few comments:
> - The numbers are not obvious as drag handles
Yeah, I was thinking about this too. Maybe adding `cursor: grab;` to the numbers would be enough of a clue?
> - The filters that are not set should just be removed, since the proposed
> design forces an default arbitrary order for the filters.
Not sure I understand this part "since the proposed design forces an default arbitrary order for the filters". But I tend to agree that filter functions that aren't set should be removed.
What if: if the filter value is empty/initial/none/invalid, then we show the default UI that's in the mockups, with the default list of functions, to let people play with them. But as soon as a valid value is set, then we just respect that value in the editor. We show the corresponding list of functions, in the right order, and that's it.
> Also, how would
> the default values for brightness would be handled ? brightness's default
> value is 100%, not 0%.
We could make the brightness slider go from 100% to 0%. But that would make it impossible to set a value of 200%, which is often what you'd want to do. Same for contrast, sometimes you want to increase the contrast by setting a value > 100%.
So, I guess we'll just have to go with the mockup:

  o------o--------------------------o
unset    0                         max

Would it be too weird if by setting the brightness (dragging the thumb out of its "unset" position) the brightness would be set to 0?
(Reporter)

Comment 25

2 years ago
Comment on attachment 8761253 [details]
css-filter-spec-sheet.png

The spec mentions that the slider should shrink to accommodate the value, but if you're using the slider to change the value, then having its size change as you use it might be odd. We'll need to make sure there's enough spacing available between the name and the slider to avoid having the value push the slider too much.

About the slider mechanism: if I get this right, there are essentially 3 important positions: unset, min, and max.
The left-most position is unset, dragging the slider all the way to the left would just remove the function.
Then we have the range defined by min and max. I think we'd have to make min and max dependent on the particular function. For instance, for brightness, saturate, sepia and contrast, maybe we want the slider to go from 0 to 200%.
But for hue-rotate, having it go from 0 to 360deg should work.
Attachment #8761253 - Flags: review?(pbrosset) → review+
(Reporter)

Comment 26

2 years ago
Here's a static prototype of the UI: https://dl.dropboxusercontent.com/u/714210/filter-editor.html
If you resize the page, the column layout updates to accommodate filter functions nicely.
You can drag the sliders to change the current filter.
You can't change the values other than by using the sliders for now.
(In reply to Patrick Brosset <:pbro> from comment #24)
> (In reply to Tim Nguyen :ntim from comment #23)
> If you're at the stage where you need to add several times the same
> function, then you probably know what you're doing and then the editor is
> probably not as helpful for you (again, sliders are nice to discover the
> feature, but not a great tool to choose specific values). At least that's
> what I understand of the reasoning.

I disagree on that. The editor should be helpful for beginners as well as advanced users. Advanced users may not use the sliders as much, though navigating within the defined filter value without the editor is somewhat cumbersome.

> It's nice because it allows us to define a much simpler and inviting UI than
> the one we have today.

It's good to have a simple UI, though I believe it should still also cover advanced (meaning all) use cases.

> > A few comments:
> > - The numbers are not obvious as drag handles
> Yeah, I was thinking about this too. Maybe adding `cursor: grab;` to the
> numbers would be enough of a clue?

This doesn't cover touch devices.

> > - The filters that are not set should just be removed, since the proposed
> > design forces an default arbitrary order for the filters.
> Not sure I understand this part "since the proposed design forces an default
> arbitrary order for the filters". But I tend to agree that filter functions
> that aren't set should be removed.
> What if: if the filter value is empty/initial/none/invalid, then we show the
> default UI that's in the mockups, with the default list of functions, to let
> people play with them. But as soon as a valid value is set, then we just
> respect that value in the editor. We show the corresponding list of
> functions, in the right order, and that's it.
> > Also, how would
> > the default values for brightness would be handled ? brightness's default
> > value is 100%, not 0%.
> We could make the brightness slider go from 100% to 0%. But that would make
> it impossible to set a value of 200%, which is often what you'd want to do.
> Same for contrast, sometimes you want to increase the contrast by setting a
> value > 100%.
> So, I guess we'll just have to go with the mockup:
> 
>   o------o--------------------------o
> unset    0                         max
> 
> Would it be too weird if by setting the brightness (dragging the thumb out
> of its "unset" position) the brightness would be set to 0?

For me it's still strange to have a special unset state in the slider. As mentioned in my previous comment, I believe it would be better to set the slider to the default value instead and interpret this as the unset state.

Sebastian
(In reply to Patrick Brosset <:pbro> from comment #24)
> (In reply to Tim Nguyen :ntim from comment #23)
> > I like the new design, however it removes the following capibilities:
> > - Adding multiple instances of the same filter
> > - Removing filters
> 
> I think the general idea is the following one: we mostly want this to be a
> way for people to discover css filters and easily play with them, without
> being scared away by having to type in complex css syntax. Sliders are
> inviting and playful. So as a user, you'd enter "filter" in the name field,
> and then would immediately be able to play around with the sliders to come
> up with some effects.
> Now, if you already have a filter defined in a rule, one that has multiple
> times the same function, or functions in a different order, or whatever,
> then opening the editor should respect that.
> The editor just won't allow you to duplicate or remove functions, but if you
> do open it from an existing filter value, then the corresponding functions
> will be shown.
> If you're at the stage where you need to add several times the same
> function, then you probably know what you're doing and then the editor is
> probably not as helpful for you (again, sliders are nice to discover the
> feature, but not a great tool to choose specific values). At least that's
> what I understand of the reasoning.
> 
> It's nice because it allows us to define a much simpler and inviting UI than
> the one we have today.
I think Patrick’s done a good job of explaining what the general goals are, but some additional context anyway:

- While I don’t consider precision and simplicity to be diametrically opposed by nature, there are times where technical precision and simplicity are at odds. Our current UI for CSS filters is technically precise but the UX is awful, full stop. This new mockup is an attempt to create something that surfaces CSS filters in a handy way without being cumbersome as a UI. Is it perfect? no. Is it a compromise? yes. Ultimately, the editors are compromises knowing that users, if they outgrow them for a particular project, can plug in more complicated values via text  in the Rules view if they so desire. For some of these editors, there isn’t prior art that captures the full complexity of CSS, so figuring out where to draw lines between that technical precision and joy-of-use UI isn’t very clear.

In the particular case of the CSS filter:
- Unset filters are removed filters. Not quite the same in that they stick aroud, but there it is.
- Multiple filters are still possible, but with the current UI you’d need to add them via the Rules view itself. How important is this functionality that we’re willing to complicate this editor for it? We’ve already made some assumptions in line with the general philosophy above by adding sliders, which aren’t technically precise for a myriad of reasons (such as many properties allowing for infinite values, brightness starting at 100, etc., etc.). Does this mean that sliders as an idea aren’t worth persuing and only technical accuracy is? Where is the line drawn for this if we consider sliders a good idea?

> > - Editing drop-shadows and urls, why can't we have `drop-shadow(_)` for now ?
> 
> I guess we could simply make the input field between the parenthesis accept
> any value, and therefore drop-shadow and url could be editable too.
We could do this, but caveat: it just won’t look good.

> > A few comments:
> > - The numbers are not obvious as drag handles
> Yeah, I was thinking about this too. Maybe adding `cursor: grab;` to the
> numbers would be enough of a clue?
I think this would be enough of a clue if `cursor: grab;` was included on the label (e.g., `blur(30px)`) as well.

> > - The filters that are not set should just be removed, since the proposed
> > design forces an default arbitrary order for the filters.
> Not sure I understand this part "since the proposed design forces an default
> arbitrary order for the filters". But I tend to agree that filter functions
> that aren't set should be removed.
I guess the argument is that I’ve chosen a default order to display them in which is arbitrary, but the user can move them around if they desire. Other than having a select box with no filters displayed like we did before I don’t know what the solution is.

> What if: if the filter value is empty/initial/none/invalid, then we show the
> default UI that's in the mockups, with the default list of functions, to let
> people play with them. But as soon as a valid value is set, then we just
> respect that value in the editor. We show the corresponding list of
> functions, in the right order, and that's it.
I agree with this, sounds like a good idea. I think that’s what I’m doing here: http://helenvholmes.com/devtools-ux/css-filter/
… although you currently can’t drag anything around.

> > Also, how would
> > the default values for brightness would be handled ? brightness's default
> > value is 100%, not 0%.
> We could make the brightness slider go from 100% to 0%. But that would make
> it impossible to set a value of 200%, which is often what you'd want to do.
> Same for contrast, sometimes you want to increase the contrast by setting a
> value > 100%.
> So, I guess we'll just have to go with the mockup:
> 
>   o------o--------------------------o
> unset    0                         max
> 
> Would it be too weird if by setting the brightness (dragging the thumb out
> of its "unset" position) the brightness would be set to 0?
Going to refer you all to this again: http://helenvholmes.com/devtools-ux/css-filter/

It’s weird, but not too terribly weird, I don’t think.


(In reply to Patrick Brosset <:pbro> from comment #25)
> Comment on attachment 8761253 [details]
> css-filter-spec-sheet.png
> 
> The spec mentions that the slider should shrink to accommodate the value,
> but if you're using the slider to change the value, then having its size
> change as you use it might be odd. We'll need to make sure there's enough
> spacing available between the name and the slider to avoid having the value
> push the slider too much.
Hm, good point. Maybe we should give enough room for three characters, and then additional characters force the area to have a horizontal scroll?

> About the slider mechanism: if I get this right, there are essentially 3
> important positions: unset, min, and max.
> The left-most position is unset, dragging the slider all the way to the left
> would just remove the function.
> Then we have the range defined by min and max. I think we'd have to make min
> and max dependent on the particular function. For instance, for brightness,
> saturate, sepia and contrast, maybe we want the slider to go from 0 to 200%.
> But for hue-rotate, having it go from 0 to 360deg should work.
Agreed, sounds correct to me. Also what I found worked after making that prototype I’ve already linked to.

(In reply to Patrick Brosset <:pbro> from comment #26)
> Here's a static prototype of the UI:
> https://dl.dropboxusercontent.com/u/714210/filter-editor.html
> If you resize the page, the column layout updates to accommodate filter
> functions nicely.
> You can drag the sliders to change the current filter.
> You can't change the values other than by using the sliders for now.
Nice.

(In reply to Sebastian Zartner [:sebo] from comment #27)
> I disagree on that. The editor should be helpful for beginners as well as
> advanced users. Advanced users may not use the sliders as much, though
> navigating within the defined filter value without the editor is somewhat
> cumbersome.
I wrote some “general philosophy” stuff earlier in the thread. I don’t have good answers for this other than that in order to move forward Patrick and I have had to make some decisions, so we’ve decided for CSS filters to cater to an audience who will be relatively new to CSS filters. Most people are, they’re not supported very well yet as of 12/6/2016: http://caniuse.com/#feat=css-filter-function

> For me it's still strange to have a special unset state in the slider. As
> mentioned in my previous comment, I believe it would be better to set the
> slider to the default value instead and interpret this as the unset state.
I found this didn’t work well as it’d be pretty normal for a user to want to set `saturate: 0` and have it be applied. Having an unset state that was separate from the zero state seemed an important distinction.
(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #28)
> Our current UI for CSS filters is technically precise but the UX is awful, full stop.

I wouldn't say awful - Mahdi already did a good job with his first implementation - but I agree that there is room for improvement.

> - Multiple filters are still possible, but with the current UI you’d need to
> add them via the Rules view itself. How important is this functionality that
> we’re willing to complicate this editor for it?

Having multiple times the same filter would add more fields to the editor, which doesn't really complicate the editor UI-wise. On the other hand, I assume it's not a common use case.
So, to answer the question this should be measured to have some numbers to help decide whether it's important or not.

> We’ve already made some
> assumptions in line with the general philosophy above by adding sliders,
> which aren’t technically precise for a myriad of reasons (such as many
> properties allowing for infinite values, brightness starting at 100, etc.,
> etc.). Does this mean that sliders as an idea aren’t worth persuing and only
> technical accuracy is? Where is the line drawn for this if we consider
> sliders a good idea?

Again, this can be answered by doing some research on how the filters are used. If the research shows that only an 	insignificant amount of websites uses filters outside the ranges of the sliders, they should stay, otherwise they may be removed.

> > > - Editing drop-shadows and urls, why can't we have `drop-shadow(_)` for now ?
> > 
> > I guess we could simply make the input field between the parenthesis accept
> > any value, and therefore drop-shadow and url could be editable too.
> We could do this, but caveat: it just won’t look good.

As mentioned in comment 17, providing text fields and a color picker where the slider is for other fields could be a solution.
You may want to add a text field within the parentheses additionally or instead, though then it should shrink on smaller UI widths to avoid breaking the layout.

> > > A few comments:
> > > - The numbers are not obvious as drag handles
> > Yeah, I was thinking about this too. Maybe adding `cursor: grab;` to the
> > numbers would be enough of a clue?
> I think this would be enough of a clue if `cursor: grab;` was included on
> the label (e.g., `blur(30px)`) as well.

And I believe the drag handles of your first iteration were more obvious, but having the grab cursor on the label (exept the input field which should have a text cursor) would probably also work in most cases.

> > > - The filters that are not set should just be removed, since the proposed
> > > design forces an default arbitrary order for the filters.
> > Not sure I understand this part "since the proposed design forces an default
> > arbitrary order for the filters". But I tend to agree that filter functions
> > that aren't set should be removed.
> I guess the argument is that I’ve chosen a default order to display them in
> which is arbitrary, but the user can move them around if they desire. Other
> than having a select box with no filters displayed like we did before I
> don’t know what the solution is.

I also don't know another solution, but I need to say the select box was not that bad. The UI may be improved by adding the selected filter immediately without having to press an 'add' button. To remove filters, a remove button should be shown when hovering and focussing a filter.

The current UI with all filters shown and no ways to add or remove them also has some advantages in allowing to play around quickly, but personally I'd prefer being able to add and remove them and it would solve the problem about the unset value more elegantly.

> > > Also, how would
> > > the default values for brightness would be handled ? brightness's default
> > > value is 100%, not 0%.
> > We could make the brightness slider go from 100% to 0%. But that would make
> > it impossible to set a value of 200%, which is often what you'd want to do.
> > Same for contrast, sometimes you want to increase the contrast by setting a
> > value > 100%.
> > So, I guess we'll just have to go with the mockup:
> > 
> >   o------o--------------------------o
> > unset    0                         max
> > 
> > Would it be too weird if by setting the brightness (dragging the thumb out
> > of its "unset" position) the brightness would be set to 0?
> Going to refer you all to this again:
> http://helenvholmes.com/devtools-ux/css-filter/
> 
> It’s weird, but not too terribly weird, I don’t think.

Again, just set the slider initially to 100% and make that the unset state.
Side effect: It helps beginners to understand the default values of the functions.

Or add back the add/remove filter functionality as mentioned above.

> (In reply to Sebastian Zartner [:sebo] from comment #27)
> > I disagree on that. The editor should be helpful for beginners as well as
> > advanced users. Advanced users may not use the sliders as much, though
> > navigating within the defined filter value without the editor is somewhat
> > cumbersome.
> I wrote some “general philosophy” stuff earlier in the thread. I don’t have
> good answers for this other than that in order to move forward Patrick and I
> have had to make some decisions, so we’ve decided for CSS filters to cater
> to an audience who will be relatively new to CSS filters. Most people are,
> they’re not supported very well yet as of 12/6/2016:
> http://caniuse.com/#feat=css-filter-function

Your link points to the wrong feature. It refers to a filter() function for <image> values.
The correct link is http://caniuse.com/#feat=css-filters, which has much better browser support.

> > For me it's still strange to have a special unset state in the slider. As
> > mentioned in my previous comment, I believe it would be better to set the
> > slider to the default value instead and interpret this as the unset state.
> I found this didn’t work well as it’d be pretty normal for a user to want to
> set `saturate: 0` and have it be applied. Having an unset state that was
> separate from the zero state seemed an important distinction.

The default value of saturate() is 100%, not 0. So, setting it to 0 *would* apply it. (Setting it to 100% would be the unset state.)

The last two points indicate that you didn't grapple with the topic enough yet. So, please be more careful when making your decisions!

Sebastian

Comment 30

2 years ago
Created attachment 8771044 [details]
macos-filter-editor.png

Just came across this filter editor on OSX (Preview app>Tools>Adjust colors). I was wondering if we could integrate these icons somehow, they seem very self-explanatory.
Flags: needinfo?(hholmes)
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #30)
> Created attachment 8771044 [details]
> macos-filter-editor.png
> 
> Just came across this filter editor on OSX (Preview app>Tools>Adjust
> colors). I was wondering if we could integrate these icons somehow, they
> seem very self-explanatory.

Hm, I'm not really as enamored as you are, Tim—do you think they're necessary to solve a problem we currently have or would you consider them an enhancement?
Flags: needinfo?(hholmes) → needinfo?(ntim.bugs)

Comment 32

2 years ago
(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #31)
> (In reply to Tim Nguyen :ntim (use needinfo?) from comment #30)
> > Created attachment 8771044 [details]
> > macos-filter-editor.png
> > 
> > Just came across this filter editor on OSX (Preview app>Tools>Adjust
> > colors). I was wondering if we could integrate these icons somehow, they
> > seem very self-explanatory.
> 
> Hm, I'm not really as enamored as you are, Tim—do you think they're
> necessary to solve a problem we currently have or would you consider them an
> enhancement?
I think it gives a better visual representation of what changing the values will do. Right now, when I set the hue-rotate filter, I don't really know what I'm doing, and I just stop sliding until my element looks right. It'd be nice to have an indication of what you're about to do when you start using the sliders, and I think those icons really help.

What do you think ?
Flags: needinfo?(ntim.bugs) → needinfo?(hholmes)
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #32)
> > Hm, I'm not really as enamored as you are, Tim—do you think they're
> > necessary to solve a problem we currently have or would you consider them an
> > enhancement?
> I think it gives a better visual representation of what changing the values
> will do. Right now, when I set the hue-rotate filter, I don't really know
> what I'm doing, and I just stop sliding until my element looks right. It'd
> be nice to have an indication of what you're about to do when you start
> using the sliders, and I think those icons really help.
> 
> What do you think ?

Like I mentioned before, I'm more ambivalent—but I can see that these might make it clearer. Tim, would you be interested in doing a first pass at creating these icons? I don't currently have the bandwidth but I'm sure we can find something we both approve of.

If you are, my current file for icons is hosted here: https://github.com/helenvholmes/design-stuff/blob/master/-Source/Icon-Generator.sketch 

All of the icons are on the "Symbols" page. I split them up into the original shape and an expanded shape which is what I ultimately minify and svgo into gecko-dev.
Flags: needinfo?(hholmes) → needinfo?(ntim.bugs)
You need to log in before you can comment on or make changes to this bug.