New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Web Inspector: Rearrange Gradient Editor to be able to show the color component input fields #2861

Merged

webkit-early-warning-system merged 1 commit into WebKit:main from patrickangle:eng/b243352

Jul 30, 2022

Contributor

patrickangle commented Jul 29, 2022 •

edited by webkit-commit-queue

`6d528bd`

Web Inspector: Rearrange Gradient Editor to be able to show the color component input fields
https://bugs.webkit.org/show_bug.cgi?id=243352
rdar://97803004

Reviewed by Devin Rousso.

Move the angle control above the color controls so that controls become more specific as you move from top to bottom of
the gradient editor. This also affords us the breathing room to show the color component fields at the bottom of the
color picker.

* Source/WebInspectorUI/UserInterface/Views/ColorPicker.js:
(WI.ColorPicker.prototype.async colorInputsWrapperElement):
(WI.ColorPicker.prototype._showColorComponentInputs):
(WI.ColorPicker.set enableColorComponentInputs): Deleted.
* Source/WebInspectorUI/UserInterface/Views/GradientEditor.css:
(.gradient-editor):
(.gradient-editor > .gradient-type-select):
(.gradient-editor > .gradient-slider):
(.gradient-editor > .color-picker):
(.gradient-editor > .gradient-angle):
(.gradient-editor.radial-gradient): Deleted.
(.gradient-editor.editing-color): Deleted.
(.gradient-editor.radial-gradient.editing-color): Deleted.
* Source/WebInspectorUI/UserInterface/Views/GradientEditor.js:
(WI.GradientEditor):

Canonical link: https://commits.webkit.org/252971@main

patrickangle requested a review from dcrousso as a code owner

July 29, 2022 21:05

patrickangle self-assigned this

patrickangle added Web Inspector WebKit Nightly Build labels

Contributor Author

patrickangle commented Jul 29, 2022

Screen.Recording.2022-07-29.at.1.51.50.PM.mov

dcrousso approved these changes

View reviewed changes

Source/WebInspectorUI/UserInterface/Views/GradientEditor.js Outdated

    
                      this._colorPicker = new WI.ColorPicker;

                      this._colorPicker.colorSquare.dimension = 195;

                      this._colorPicker.enableColorComponentInputs = false;

Member

dcrousso Jul 29, 2022

oops :P

Source/WebInspectorUI/UserInterface/Views/ColorPicker.js

    
                  set enableColorComponentInputs(value)

                  {

                      this._enableColorComponentInputs = value;

                      this._element.classList.toggle("hide-inputs", !this._enableColorComponentInputs);

Member

dcrousso Jul 29, 2022

can we also get rid of the CSS .color-picker.hide-inputs?

Source/WebInspectorUI/UserInterface/Views/GradientEditor.css Outdated

    
            @@ -24,25 +25,13 @@
          
               */

              .gradient-editor {

                  --gradient-editor-controls-width:  237px;

Member

dcrousso Jul 29, 2022

Style: should only have one space after :

NIT: I usually put CSS variables at the bottom of a declaration so that the actual things that affect style are shown first (similar to how we put member variables at the bottom of a class in C++)

Source/WebInspectorUI/UserInterface/Views/GradientEditor.css

    
                  padding-bottom: 15px;

              }

              .gradient-editor.editing-color {

Member

dcrousso Jul 29, 2022

it seems like we could also get rid of "editing-color" in the JS too?

Source/WebInspectorUI/UserInterface/Views/GradientEditor.css Outdated

    
              }

              .gradient-editor > .gradient-type-select {

                  width: 237px;

                  width: var(--gradient-editor-controls-width);

Member

dcrousso Jul 29, 2022

would width: 100% not work here?

Source/WebInspectorUI/UserInterface/Views/GradientEditor.css Outdated

    
              }

              .gradient-editor > .color-picker {

                  width: 238px;

                  width: calc(var(--gradient-editor-controls-width) + 1px);

Member

dcrousso Jul 29, 2022

ditto (:34)

Source/WebInspectorUI/UserInterface/Views/GradientEditor.css Outdated

    
            @@ -65,12 +54,11 @@
          
              }

              .gradient-editor > .gradient-angle {

                  width: var(--gradient-editor-controls-width);

Member

dcrousso Jul 29, 2022

ditto (:34)

patrickangle force-pushed the eng/b243352 branch from f5d0138 to 006b84b Compare

July 29, 2022 22:20

patrickangle added the merge-queue label


          Web Inspector: Rearrange Gradient Editor to be able to show the color…

6d528bd

… component input fields

https://bugs.webkit.org/show_bug.cgi?id=243352
rdar://97803004

Reviewed by Devin Rousso.

Move the angle control above the color controls so that controls become more specific as you move from top to bottom of
the gradient editor. This also affords us the breathing room to show the color component fields at the bottom of the
color picker.

* Source/WebInspectorUI/UserInterface/Views/ColorPicker.js:
(WI.ColorPicker.prototype.async colorInputsWrapperElement):
(WI.ColorPicker.prototype._showColorComponentInputs):
(WI.ColorPicker.set enableColorComponentInputs): Deleted.
* Source/WebInspectorUI/UserInterface/Views/GradientEditor.css:
(.gradient-editor):
(.gradient-editor > .gradient-type-select):
(.gradient-editor > .gradient-slider):
(.gradient-editor > .color-picker):
(.gradient-editor > .gradient-angle):
(.gradient-editor.radial-gradient): Deleted.
(.gradient-editor.editing-color): Deleted.
(.gradient-editor.radial-gradient.editing-color): Deleted.
* Source/WebInspectorUI/UserInterface/Views/GradientEditor.js:
(WI.GradientEditor):

Canonical link: https://commits.webkit.org/252971@main

webkit-early-warning-system force-pushed the eng/b243352 branch from 006b84b to 6d528bd Compare

July 30, 2022 00:05

Collaborator

webkit-commit-queue commented Jul 30, 2022

Committed 252971@main (6d528bd): https://commits.webkit.org/252971@main

Reviewed commits have been landed. Closing PR #2861 and removing active labels.

webkit-early-warning-system merged commit 6d528bd into WebKit:main

webkit-commit-queue removed the merge-queue label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment