Open Bug 1272148 Opened 9 years ago Updated 2 years ago

Border Inline Editor

Categories

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

enhancement

Tracking

(Not tracked)

People

(Reporter: hholmes, Unassigned)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 5 obsolete files)

Attached image border-inline-editor-spec.png (obsolete) (deleted) —
Let's start the discussion on the second inline editor—a border editor!
Depends on: 1258390
Priority: -- → P2
Comment on attachment 8751487 [details] border-inline-editor-spec.png Patrick, would you mind glancing through this?
Attachment #8751487 - Flags: review?(pbrosset)
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?
Attachment #8751487 - Flags: review?(pbrosset) → feedback+
Attached image border-spec.png (obsolete) (deleted) —
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.
Attachment #8751487 - Attachment is obsolete: true
Attachment #8753421 - Flags: review?(pbrosset)
Attached image border-spec.png (obsolete) (deleted) —
Whoops, a few comment lines went to nowhere.
Attachment #8753421 - Attachment is obsolete: true
Attachment #8753421 - Flags: review?(pbrosset)
Attachment #8753428 - Flags: review?(pbrosset)
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.
Attached image border-inline-editor-spec.png (obsolete) (deleted) —
Attachment #8753428 - Attachment is obsolete: true
Attachment #8753428 - Flags: review?(pbrosset)
Attachment #8753916 - Flags: review?(pbrosset)
Attached image border-inline-editor-spec.png (obsolete) (deleted) —
I think we forgot border-radius D:
Attachment #8753916 - Attachment is obsolete: true
Attachment #8753916 - Flags: review?(pbrosset)
Attachment #8753987 - Flags: review?(pbrosset)
(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 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.
Attachment #8753987 - Flags: review?(pbrosset) → review+
Attached image border-spec-sheet.png (deleted) —
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!
Attachment #8753987 - Attachment is obsolete: true
Attachment #8755272 - Flags: review?(pbrosset)
Comment on attachment 8755272 [details] border-spec-sheet.png Looks great! Thanks Helen.
Attachment #8755272 - Flags: review?(pbrosset) → review+
Blocks: 1258390
No longer depends on: 1258390
Severity: normal → enhancement
Component: Developer Tools: Inspector → Developer Tools: CSS Rules Inspector
Product: Firefox → DevTools
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: