Show a warning for invalid declarations and filter icon for overridden declarations in the new rules view
Categories
(DevTools :: Inspector: Rules, enhancement, P3)
Tracking
(firefox66 fixed)
| Tracking | Status | |
|---|---|---|
| firefox66 | --- | fixed |
People
(Reporter: gl, Assigned: gl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
|
6.08 KB,
patch
|
mtigley
:
review+
|
Details | Diff | Splinter Review |
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
|
||
Comment 3•5 years ago
|
||
Comment on attachment 9037727 [details] [diff] [review] 1521243.patch [1.0] Review of attachment 9037727 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me. One question though: could we add a test for this? Or will this be done in a separate bug? ::: devtools/client/inspector/rules/reducers/rules.js @@ +33,5 @@ > // An unique CSS declaration id. > id: declaration.id, > + // Whether or not the declaration is valid. (Does it make sense for this value > + // to be assigned to this property name?) > + isDeclarationValid: declaration.isValid(), This makes sense to me.
|
|
Assignee | |
Comment 4•5 years ago
|
||
(In reply to Micah Tigley [:mtigley] from comment #3)
Comment on attachment 9037727 [details] [diff] [review]
1521243.patch [1.0]Review of attachment 9037727 [details] [diff] [review]:
This looks good to me. One question though: could we add a test for this? Or
will this be done in a separate bug?::: devtools/client/inspector/rules/reducers/rules.js
@@ +33,5 @@// An unique CSS declaration id. id: declaration.id,
- // Whether or not the declaration is valid. (Does it make sense for this value
- // to be assigned to this property name?)
- isDeclarationValid: declaration.isValid(),
This makes sense to me.
We would be ideally be reusing the existing tests in the rules view, which will covers this. The plan is to get the frontend working and adjust all the existing unit tests pass on the new frontend then flip the switch to the new frontend.
|
|
||
Comment 5•5 years ago
|
||
(In reply to Gabriel [:gl] (ΦωΦ) from comment #4)
(In reply to Micah Tigley [:mtigley] from comment #3)
Comment on attachment 9037727 [details] [diff] [review]
1521243.patch [1.0]Review of attachment 9037727 [details] [diff] [review]:
This looks good to me. One question though: could we add a test for this? Or
will this be done in a separate bug?::: devtools/client/inspector/rules/reducers/rules.js
@@ +33,5 @@// An unique CSS declaration id. id: declaration.id,
- // Whether or not the declaration is valid. (Does it make sense for this value
- // to be assigned to this property name?)
- isDeclarationValid: declaration.isValid(),
This makes sense to me.
We would be ideally be reusing the existing tests in the rules view, which will covers this. The plan is to get the frontend working and adjust all the existing unit tests pass on the new frontend then flip the switch to the new frontend.
Sounds good to me. Thanks for clarifying!
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/65e0b8a04d1a Show a warning for invalid declarations and filter icon for overridden declarations in the new rules view. r=mtigley
|
||
Comment 7•5 years ago
|
||
| bugherder | ||
Description
•