Closed Bug 1240670 Opened 9 years ago Closed 9 years ago

Hide the filter property search for an unmatched rule

Categories

(DevTools :: Inspector: Rules, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: gl, Assigned: gl)

Details

Attachments

(1 file, 1 obsolete file)

The filter property search should be hidden when a rule does not match the currently selected rule. STR: 1. Edit selector to a selector that doesn't match the current element 2. Assuming there is a property that is overridden in the new rule created, see the filter property search appears
Attached patch 1240670.patch (obsolete) — Splinter Review
Attachment #8709301 - Flags: feedback?(pbrosset)
Comment on attachment 8709301 [details] [diff] [review] 1240670.patch Review of attachment 8709301 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me.
Attachment #8709301 - Flags: feedback?(pbrosset) → feedback+
Comment on attachment 8709850 [details] [diff] [review] 1240670.patch [1.0] Review of attachment 8709850 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/inspector/rules/views/rule-editor.js @@ +494,5 @@ > ruleView.emit("ruleview-invalid-selector"); > return; > } > > + ruleProps.isUnmatched = !isMatching; I don't feel too strongly about this, but it might be cleaner to do this in the Front class instead. I can imagine someone reading through this code for the first time thinking: "'this.rule.domRule.modifySelector' is a server request that returns a ruleProps object that's really useful, and I can see it contains things like 'isSystem'. So why is 'isUnmatched' not already a property of ruleProps? Why is it sent separately and why do I have have to set it manually upon receiving the response?"
Attachment #8709850 - Flags: review?(pbrosset) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: