Closed Bug 1381692 Opened 4 years ago Closed 4 years ago

Reorder and rename the Grid Display Settings checkboxes

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: gl, Assigned: gl)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Reorder the Grid Display Settings to be the following:

Display line numbers
Display area names
Display line names
Extend lines infinitely
Blocks: dt-grid
Summary: Reorder the Grid Display Settings checkboxes → Reorder and rename the Grid Display Settings checkboxes
This is actually reordering and renaming some of the Grid display settings checkboxes to be the following:

Display line numbers
Display area names
Extend lines infinitely
Comment on attachment 8887279 [details]
Bug 1381692 - Reorder and rename the Grid display settings checkboxes.

https://reviewboard.mozilla.org/r/158078/#review163260

::: devtools/client/inspector/grids/components/GridDisplaySettings.js:82
(Diff revision 1)
>                  type: "checkbox",
> -                checked: highlighterSettings.showInfiniteLines,
> -                onChange: this.onShowInfiniteLinesCheckboxClick,
> +                checked: highlighterSettings.showGridLineNumbers,
> +                onChange: this.onShowGridLineNumbersCheckboxClick,
>                }
>              ),
> -            getStr("layout.extendGridLinesInfinitely")
> +            getStr("layout.displayNumbersOnLines2")

s/displayNumbersOnLines2/displayNumbersOnLines

::: devtools/client/inspector/grids/components/GridDisplaySettings.js:99
(Diff revision 1)
> -                type: "checkbox",
> +               type: "checkbox",
> -                checked: highlighterSettings.showGridLineNumbers,
> -                onChange: this.onShowGridLineNumbersCheckboxClick,
> +               checked: highlighterSettings.showGridAreasOverlay,
> +               onChange: this.onShowGridAreasCheckboxClick,
> -              }
> +             }
> -            ),
> +           ),
> -            getStr("layout.displayNumbersOnLines2")
> +           getStr("layout.displayGridAreas2")

s/displayGridAreas2/displayGridAreas

::: devtools/client/inspector/grids/components/GridDisplaySettings.js:118
(Diff revision 1)
> -             }
> +              }
> -           ),
> +            ),
> -           getStr("layout.displayGridAreas")
> +            getStr("layout.extendGridLinesInfinitely2")
> -          )
> -        )
> +          )
> +        ),

Remove trailing comma: Eslint configuration does not accept trailing commas.

::: devtools/client/locales/en-US/layout.properties:13
(Diff revision 1)
>  # LOCALIZATION NOTE (layout.cannotShowGridOutline, layout.cannotSHowGridOutline.title):
>  # In the case where the grid outline cannot be effectively displayed.
>  layout.cannotShowGridOutline=Cannot show outline for this grid
>  layout.cannotShowGridOutline.title=The selected grid’s outline cannot effectively fit inside the layout panel for it to be usable.
>  
> -# LOCALIZATION NOTE (layout.displayGridAreas): Label of the display grid areas setting
> +# LOCALIZATION NOTE (layout.displayGridAreas2): Label of the display grid areas setting

s/displayGridAreas2/displayGridAreas

::: devtools/client/locales/en-US/layout.properties:13
(Diff revision 1)
>  # LOCALIZATION NOTE (layout.cannotShowGridOutline, layout.cannotSHowGridOutline.title):
>  # In the case where the grid outline cannot be effectively displayed.
>  layout.cannotShowGridOutline=Cannot show outline for this grid
>  layout.cannotShowGridOutline.title=The selected grid’s outline cannot effectively fit inside the layout panel for it to be usable.
>  
> -# LOCALIZATION NOTE (layout.displayGridAreas): Label of the display grid areas setting
> +# LOCALIZATION NOTE (layout.displayGridAreas2): Label of the display grid areas setting

Label of the display grid areas's names setting

::: devtools/client/locales/en-US/layout.properties:15
(Diff revision 1)
>  layout.cannotShowGridOutline=Cannot show outline for this grid
>  layout.cannotShowGridOutline.title=The selected grid’s outline cannot effectively fit inside the layout panel for it to be usable.
>  
> -# LOCALIZATION NOTE (layout.displayGridAreas): Label of the display grid areas setting
> +# LOCALIZATION NOTE (layout.displayGridAreas2): Label of the display grid areas setting
>  # option in the CSS Grid pane.
> -layout.displayGridAreas=Display grid areas
> +layout.displayGridAreas2=Display area names

Same as above

::: devtools/client/locales/en-US/layout.properties:21
(Diff revision 1)
>  
>  # LOCALIZATION NOTE (layout.displayNumbersOnLines2): Label of the display numbers on lines
>  # setting option in the CSS Grid pane.
>  layout.displayNumbersOnLines2=Display line numbers
>  
> -# LOCALIZATION NOTE (layout.extendGridLinesInfinitely): Label of the extend grid lines
> +# LOCALIZATION NOTE (layout.extendGridLinesInfinitely2): Label of the extend grid lines

s/extendGridLinesInfinitely2/extendGridLinesInfinitely

::: devtools/client/locales/en-US/layout.properties:23
(Diff revision 1)
>  # setting option in the CSS Grid pane.
>  layout.displayNumbersOnLines2=Display line numbers
>  
> -# LOCALIZATION NOTE (layout.extendGridLinesInfinitely): Label of the extend grid lines
> +# LOCALIZATION NOTE (layout.extendGridLinesInfinitely2): Label of the extend grid lines
>  # infinitely setting option in the CSS Grid pane.
> -layout.extendGridLinesInfinitely=Extend grid lines infinitely
> +layout.extendGridLinesInfinitely2=Extend lines infinitely

Same as above
Attached patch 1381692.patch (obsolete) — Splinter Review
Attachment #8887279 - Attachment is obsolete: true
Attachment #8887279 - Flags: review?(tigleym)
Attachment #8887319 - Flags: review?(tigleym)
Comment on attachment 8887319 [details] [diff] [review]
1381692.patch

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

::: devtools/client/inspector/grids/components/GridDisplaySettings.js
@@ +95,5 @@
>                 checked: highlighterSettings.showGridAreasOverlay,
>                 onChange: this.onShowGridAreasCheckboxClick,
>               }
>             ),
> +           getStr("layout.displayGridAreas2")

s/displayGridAreas2/displayAreaNames
Attachment #8887319 - Attachment is obsolete: true
Attachment #8887319 - Flags: review?(tigleym)
Attachment #8887323 - Flags: review?(tigleym)
Attachment #8887323 - Flags: review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eef6321e071e
Reorder and rename the Grid display settings checkboxes. r=micah
https://hg.mozilla.org/mozilla-central/rev/eef6321e071e
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
-layout.displayNumbersOnLines2=Display line numbers
+layout.displayLineNumbers=Display line numbers

Next time, please let's not change a string ID when the string doesn't change, unless there are very good reasons for doing it (and this doesn't seem to be the case).
This Feature is implemented in Latest Nightly 

Build ID   :20170721030204
User Agent :Mozilla/5.0 (Windows NT 6.1; rv:56.0) Gecko/20100101 Firefox/56.0

Tested OS-- windows7 32bit
QA Whiteboard: [testday-20170721]
Attachment #8887323 - Flags: review?(tigleym)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.