Highlight changed items in the rule view

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Inspector
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Kevin Dangoor, Assigned: miker)

Tracking

unspecified
Firefox 15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ruleview][firefox14-wanted][fixed-in-fx-team])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
To make it easy for a user to see what they've changed, we should visually distinguish the items in the rules view (and possibly the computed view as well)

Comment 1

6 years ago
I understand for the computed view, but for the rule view, how is it useful? What kind of changes are you talking about?
The only place that you can make changes to the css are the rule view and style editors so it doesn't really make sense to have this for the computed view.

We think that highlighting changes using a green line down the gutter makes the most sense as this is how most code editors do it.

As far as implementation ... we currently save user entered changes in a store so highlighting the rows would be very simple as we are already keeping track of changed property values.

It is literally an hours work.
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Whiteboard: [ruleview]

Comment 3

6 years ago
(In reply to Michael Ratcliffe from comment #2)
> The only place that you can make changes to the css are the rule view and
> style editors so it doesn't really make sense to have this for the computed
> view.

I am not sure to understand. We are talking about a value being updated, right?

In the Rule View and the Computed Style, when does a value change?
When you edit the style in the Style Editor or in the Rule View, right?

Why not updating the Computed View while using the Style Editor?
(Reporter)

Comment 4

6 years ago
(In reply to Paul Rouget [:paul] from comment #1)
> I understand for the computed view, but for the rule view, how is it useful?
> What kind of changes are you talking about?

The specific use case here is that the web developer is working on their design and they make a bunch of changes. Then, they want to get those changes into their original source files.

If we make the items they've changed visible, that will make it easier for them to make the changes in their original source.

In the Style Editor, they can save pretty easily (unless they're not editing the original source... eg they're working from minified files or originally had LESS/SASS). In the rule view, it's a bit harder.

(A global "diff" view that enumerates the changes they've made may be even more helpful. This is just one option.)

Comment 5

6 years ago
hoooo, I see. I was totally off track :) Forget about my comments.

Updated

6 years ago
Whiteboard: [ruleview] → [ruleview][firefox14-wanted]

Updated

6 years ago
Assignee: nobody → mratcliffe
Created attachment 615636 [details] [diff] [review]
Patch

Very simple patch
Attachment #615636 - Flags: review?(mihai.sucan)
Comment on attachment 615636 [details] [diff] [review]
Patch

Review of attachment 615636 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! Thanks for the patch!

Just a couple of comments below (addressing these is not mandatory for the review).

::: browser/devtools/styleinspector/CssRuleView.jsm
@@ +440,5 @@
>     * style's store.  Will re-mark overridden properties.
> +   *
> +   * @param {string} [aName]
> +   *        A text property name (such as "background" or "border-top") used
> +   *        when calling from setPropertyValue & setPropertyName to signify that

s/signify/flag/

(not sure, but doesn't "flag" fit better in this case?)

@@ +460,5 @@
>        }
>  
> +      if (aName && prop.name == aName) {
> +        store.userProperties.setProperty(this.style, prop.name, prop.value);
> +      }

This looks fine to me, but I wonder if this stuff would fit better in the TextProperty.setName/setValue() methods? What do you think?

I mean, why should we keep adding stuff to applyProperties()? Maybe we can keep it smaller.

Just a thought...
Attachment #615636 - Flags: review?(mihai.sucan) → review+
Whiteboard: [ruleview][firefox14-wanted] → [ruleview][firefox14-wanted][land-in-fx-team]

Comment 8

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/316c8e95720f
Whiteboard: [ruleview][firefox14-wanted][land-in-fx-team] → [ruleview][firefox14-wanted][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/316c8e95720f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
You need to log in before you can comment on or make changes to this bug.