1272148 - Border Inline Editor

Status: NEW

(DevTools :: Inspector: Rules, enhancement, P2)

Description

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-inline-editor-spec.png

Let's start the discussion on the second inline editor—a border editor!

Comment 1

[Helen V. Holmes (:helenvholmes) (:✨)]

Comment on attachment 8751487 [details]
border-inline-editor-spec.png

Patrick, would you mind glancing through this?

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8751487 [details]
border-inline-editor-spec.png

Thanks Helen, this looks really nice.
Here are a few comments:

- Border is a special shorthand which, unlike the corresponding longhands, doesn't allow to set different values for all 4 sides. Only a single value is used. So you can't do something like
border: 1px 2px 1em 1px solid dashed dotted solid red blue yellow tomato;
This raises the first important question of inline editors: should they be able to add new properties (also described in bug 1258390 comment 2)?
For instance, if you start with
border: 1px solid red;
and then select a different width (2px) for the top side, we should end up with:
border: 1px solid red;
border-top-width: 2px;
I think this is going to be needed for several inline editors anyway.

- Not sure how important the ordering is, but sides in CSS normally in this order: top, right, bottom, left. So perhaps we should order the checkboxes this way too.

- Is the accordion expand icon supposed to be so small, or is it just that the image was scaled down before saving? I can hardly see if it's a triangle or just a dot on my screen. I'm guessing it's a triangle because clicking on it expands the list of sides, but not sure.

- What do you think about having just one layout instead of the 2 and 3 column layouts? In the 2 columns layout, I like how width and style are up top, makes it easy to see them. I feel like in the 3-columns layout they are a bit lost.
I understand that we should use the available space though.
By the way, we'll need to find out how we want to handle cases where the rule-view is made super small. Should there be a 1 column version too? 
Should there be min-width?
Also, if the rule-view is super wide, do we want to extend the various fields so they take up the unused space, or just center the whole editor and have empty margins on the side?

Comment 3

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-spec.png

Thanks for going over these Patrick!

(In reply to Patrick Brosset <:pbro> from comment #2)
> Comment on attachment 8751487 [details]
> border-inline-editor-spec.png
> So you can't do something like
> border: 1px 2px 1em 1px solid dashed dotted solid red blue yellow tomato;
> This raises the first important question of inline editors: should they be
> able to add new properties (also described in bug 1258390 comment 2)?
So I think that some inline editors (probably will depend on the editor) should add new properties. In the newest spec I have a comment about it that's also here: http://cl.ly/0Z402b061F0F

My only real concern with the adding/removing of props is the relationship between the ^ arrow and the ⬛ that opens it. If properties that the inline editor could be added _above_ the css line that opened it, we could preserve that relationship—but I'm worried this could have some css inheritance issues? This, for example, might not be rendered the way the user hoped it would be?

> border-top-width: 5px;
> border: 2px solid tomato;

> - Not sure how important the ordering is, but sides in CSS normally in this
> order: top, right, bottom, left. So perhaps we should order the checkboxes
> this way too.
Whoops—yep, we should definitely order things this way. My mistake! 

> - Is the accordion expand icon supposed to be so small, or is it just that
> the image was scaled down before saving? I can hardly see if it's a triangle
> or just a dot on my screen. I'm guessing it's a triangle because clicking on
> it expands the list of sides, but not sure.
I made it larger, it was pretty tiny.

> - What do you think about having just one layout instead of the 2 and 3
> column layouts? In the 2 columns layout, I like how width and style are up
> top, makes it easy to see them. I feel like in the 3-columns layout they are
> a bit lost.
Tell me if you still feel this way—I also updated the designs to have 1, 2, and 3 column designs. I think that having all three would be nice for the responsive resizing. I think I too would prefer the two-column to be the "default" for the size most people have their Rules view opened to.

> Should there be min-width?
Yeah—should probably just be the min-width that the Rules view has set, though.

> Also, if the rule-view is super wide, do we want to extend the various
> fields so they take up the unused space, or just center the whole editor and
> have empty margins on the side?
I think we might just left-align it—I mocked that up. Looks.... eh, it looks weird, but then again, I suspect all of our inline editors will start to look odd at a certain size.

Comment 4

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-spec.png

Whoops, a few comment lines went to nowhere.

Comment 5

[Helen V. Holmes (:helenvholmes) (:✨)]

Something another designer mentioned to me that I hadn't known: border-width acts as a shorthand, so in this scenario http://cl.ly/0Z402b061F0F we can potentially just have one shorthand property for border widths (border-width: 5px 5px 2px 2px) instead of the longhand. Just a thought.

Comment 6

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-inline-editor-spec.png

Comment 7

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-inline-editor-spec.png

I think we forgot border-radius D:

Comment 8

[Patrick Brosset <:pbro>]

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #3)
> (In reply to Patrick Brosset <:pbro> from comment #2)
> > Comment on attachment 8751487 [details]
> > border-inline-editor-spec.png
> > So you can't do something like
> > border: 1px 2px 1em 1px solid dashed dotted solid red blue yellow tomato;
> > This raises the first important question of inline editors: should they be
> > able to add new properties (also described in bug 1258390 comment 2)?
> So I think that some inline editors (probably will depend on the editor)
> should add new properties. In the newest spec I have a comment about it
> that's also here: http://cl.ly/0Z402b061F0F
> 
> My only real concern with the adding/removing of props is the relationship
> between the ^ arrow and the ⬛ that opens it. If properties that the inline
> editor could be added _above_ the css line that opened it, we could preserve
> that relationship—but I'm worried this could have some css inheritance
> issues? This, for example, might not be rendered the way the user hoped it
> would be?
Yeah,
border-width: 1px 2px 1px 1px;
border: 1px solid red;
will *not* end up defining a 2px right border, because the 2px gets overridden by the border shorthand property value.

This would work though:
border: 1px solid red;
border-width: 1px 2px 1px 1px;

but as you said, the inline editor is sort of disconnected from the anchor it was opened from in the first place.

Maybe we could visually group these properties so they somehow feel all connected to the inline editor? Maybe have a different brackground?

However, the other problem is if the rule already contains several border-related properties before you open the editor, like:

border: ⬛ 1px solid red;
border-color: red red blue red;

Clicking on the button to open the inline editor should really take into consideration that there is the a border-color property defined in this rule already.
Plus, this property may be defined anywhere else in this rule, so visually indicating that the inline editor actually controls both is going to be challenging.
What if you have:

border: ⬛ 1px solid red;
float: left;
background: red;
font-family: arial;
border-color: red red blue red;

It becomes even worse when those properties are scattered around in several rules like:

div#id {
  border-color: red;
}
div {
  border: ⬛ 1px solid blue;
}

All DIVs have a blue color, except the one that has the ID id. So, in this case, if you start using the inline editor where the ⬛ is, you won't see the element border color change in the page since there's another, more specific, rule that defines it to be red, and that's not what the inline editor is currently changing.

I think we're going to have to choose carefully what we want and don't want this editor to do, and choose it so that it always feels natural to users, so they know what to expect.

I'm starting to think that maybe we shouldn't generate/edit other properties.
This particular inline editor's role is to ease writing/editing a border value, and whether or not there is a more specific declaration somewhere else that overrides what you're editing doesn't change that fact. You're still using something visual with nice drop-downs and color-pickers to change a value.
We want users to understand inheritance and specificity, not have a tool that magically generates CSS people won't understand just for the sake of being a wysiwyg.
That's the balance we have to find, and it's not easy. But I feel like as soon as we try and do too much, we're going to be bitten by edge cases that are just going to be too hard to solve.

So I think we should have this editor only care about these properties:
border/border-top/border-right/border-bottom/border-left
All of them accept the following syntax: size [style] [color]
Just one size, one style, one color.

And bonus points if we can have the editor warn users when there are other properties that override all or part.
At least if we only ever update the value that's next to the ⬛ button, then people will always know what to expect.

This would simplify the editor quite a bunch too. For instance, no need to control the border-radius. Border-radius btw can be very complex. There can be a different number for each corner, and even different horizontal and vertical values!
Maybe we should even have a separate inline editor just for border-radius (which reminds me, I had done some kind of a visualization tool for it: http://captainbrosset.github.io/radius-inspector/ ).

Sorry for the very long comment. Hopefully it makes sense. Let me know what you think.

Comment 9

[Patrick Brosset <:pbro>]

Comment on attachment 8753987 [details]
border-inline-editor-spec.png

Looks good to me (obviously, considering my previous comment, there could be some changes to the width editor).
I really like the line style preview. In fact, having it reflect the width, style and color could be a nice way to let users see what it looks like, even when some other properties override it and make it look different in content.

Comment 10

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: border-spec-sheet.png

Some final changes based on what we discussed over IRC. I know you r+'d the last one, but if you have time to review this anyway that'd be great!

Comment 11

[Patrick Brosset <:pbro>]

Comment on attachment 8755272 [details]
border-spec-sheet.png

Looks great! Thanks Helen.