Fix for thimble.mozilla.org#2508 - Quick Edit UI for box-shadow property #888

Cryzek · 2017-10-20T09:03:20Z

Related to mozilla/thimble.mozilla.org#2508 for creating an extension to open quick edit UI for box-shadow property( similar to color-picker).
This is a very early version with bugs, and lot of unnecessary comments, logs. So, it's not the most robust one.

It's a work in progress.

@humphd @flukeout I have a lot of doubts on this one -

Color picker - Even when in the box-shadow property, when cursor is over the color value, quick edit opens editor for color-picker instead of box-shadow one. Although it works to my advantage of not integrating an color-picker inside the box-shadow editor.
Multiple box-shadows - I can't get an idea of how to implement an editor for multiple box-shadows. eg. box-shadow: 1px 1px black, -1px -1px grey
Should inputs be replaced with sliders?

flukeout · 2017-10-20T22:40:27Z

Hey team - I'll be a bit delayed in checking this out. Just moved to a new machine and my Github stuff seems broken.

flukeout · 2017-10-20T22:53:43Z

Hey this is great! I'll provide some more feedback later. One thing I noticed is that if you are editing a value in one of the input boxes, if you backspace to delete a value like 2 it turns into 0 and then the cursor is placed behind the 0 so you end up typing a new value like 03 when all you wanted was 3. Do you know what I mean? It would be nice to be able to erase the value totally. Maybe if you leave the field blank and move away from the input, we can pick a default value to use.

By the way, great job getting this running! These inline editors are tricky. I'll give some thought to how we can treat overlapping editors. cc @gideonthomas @humphd do you have any thoughts? Maybe the little Icon could indicate the type of editor that it will pop up, so the user knows what to expect in ambiguous cases. Is it possible to set a priority order for the editors?

humphd

I'll do a more thorough review after you fix the indents throughout this patch. It's looking like you're making it really far into this, nice job.

Question: what does tinycolor-min do? I wonder if we already have this type of code available in Brackets core.

humphd · 2017-10-21T16:25:29Z

src/extensions/default/InlineBoxShadowEditor/BoxShadowEditor.js

@@ -0,0 +1,268 @@
+define(function(require, exports, module) {


Can you fix all new files to use 4-space indents please? There are some other indent issues below as well.

tinycolor-min is used in the InlineColorEditor at a stage for detecting if the color value is valid.
I didn't get the complete hang of it's but I used it anyway.

Cryzek · 2017-10-21T17:26:54Z

@flukeout Consider that a current value is - box-shadow: 5px 5px 5px 1px black,and the user edit's the blur value in the editor(box-shadow quick edit UI) by pressing backspace(so as to want to remove blur).

Right now this would change the string to box-shadow: 5px 5px 1px black. As a result the spread value would be assigned to the blur value. So, to prevent it, I set it to 0px.

flukeout · 2017-11-13T22:44:34Z

Sorry for the delay @Cryzek - am back in action now.

Is it possible to replace the value with 0 when you blur the input field? It's just really cumbersome right now when trying to change a 0 value to something like 3.

We could do this maybe by replacing the value in the CSS with 0 when the input field is blank, and then filling in the field with the 0 when you leave the input field?

However

If we replace the text inputs with sliders, we won't have to deal with this anymore since there will be no text editing of the values. What do you say to that?

flukeout · 2017-11-13T22:47:44Z

Another question, is it possible to show the Purple Indicator when we don't have a value for the property yet? Like, when the user types box-shadow:

Cryzek · 2017-11-14T16:37:32Z

@flukeout Thanks for the feeback.
I guess sliders would altogether avoid the current problem.
About sliders, is it ok to limit the range values?
What if the user types in a value larger than the fixed range, and then opens the quick-edit ui?

For the second comment, yes it is.
I changed the code to open the quick edit UI if the line necessarily contains box-shadow since a same value (eg. 2px 2px 2px 2px black) could be used within border property.

humphd · 2017-12-04T18:44:24Z

@Cryzek how is this coming?

Cryzek · 2017-12-05T14:22:03Z

I'm sorry I haven't able to work on this for a while now. I had made a few changes earlier but after that I haven't worked on it further. Can I let you know in 2-3 days?

humphd · 2017-12-06T15:47:35Z

@Cryzek yes, that's fine. I just wanted to check in.

Related to thimble.mozilla.org#2508 for creating an extension to open quick edit UI for box-shadow property( similar to color-picker). This is a very early version with bugs, and lot of unnecessary comments, logs. It's a work in progress.

Cryzek · 2017-12-19T17:22:32Z

@flukeout I have made this working with input sliders. Can I use your css code from the ongoing work on border-radius quick edit by feihaozi77?

Related to thimble.mozilla.org#2508 for creating an extension to open quick edit UI for box-shadow property( similar to color-picker). This is a very early version with bugs, and lot of unnecessary comments, logs. It's a work in progress.

Rewrote most of the code from a huge procedural chunk to objects of type BoxShadowInput viz. BoxShadowLength, BoxShadowColor. The new responsibilities are - + InlineBoxShadowEditor - It initialises the widget and acts as intermediary for propagating values between text editor and box-shadow editor + BoxShadowEditor - The overall manager for different BoxShadowInputs. Gets values from InlineBoxShadowEditor and passes them to the appropriate BoxShadowInput. +BoxShadowLength - BoxShadowInput for length types +BoxShadowColor - BoxShadowInput for color value

…-issue-2508

…l.iterator to simple counter over array

Cryzek · 2018-03-08T14:02:29Z

@humpd Could you take a look?

Earlier, the BoxShadowEditor, BoxShadowLength, BoxShadowColor were importing and using the complete exports objects. Fixed by requiring only the component. eg. changed `require("BoxShadowInput")` to `require("BoxShadowInput").BoxShadowInput`;

Created BoxShadowInset that responds and provides control over `inset` property in box-shadow

…e box-shadow editor InlineBoxShadowEditor uses a `orderOfValues` to preseve the user provided order of values while building the boxShadowString when changes are made in widget. Note: Changes to BoxShadowEditorTemplate.html - renaming `slider-container` to `color-container` for color input Changes to style.less - translation of toggle by amount of its width

Cryzek · 2018-03-10T19:04:47Z

Hi @humphd @flukeout. Sorry for the huge delays.
I have finally been able to get this working. The box-shadow editor allows control over the inset property of box-shadow, and the order of values provided by the user are preserved while making changes in the UI editor.
I'll start working on the comments and localization, if the work up to now looks good.

gideonthomas requested review from humphd and flukeout October 20, 2017 21:49

humphd suggested changes Oct 21, 2017

View reviewed changes

Cryzek added 5 commits December 8, 2017 19:43

Working quick-edit UI for box-shadow property

cfd0563

Related to thimble.mozilla.org#2508 for creating an extension to open quick edit UI for box-shadow property( similar to color-picker). This is a very early version with bugs, and lot of unnecessary comments, logs. It's a work in progress.

Fixes eslint errors in InlineBoxShadowEditor

cbd036f

Changed indent tabs to 4 spaces

926e0d6

Changed input textboxes to input range

bb10ee6

Fixed eslint errors

207d69b

Cryzek and others added 11 commits February 20, 2018 14:51

Working quick-edit UI for box-shadow property

d8083ab

Related to thimble.mozilla.org#2508 for creating an extension to open quick edit UI for box-shadow property( similar to color-picker). This is a very early version with bugs, and lot of unnecessary comments, logs. It's a work in progress.

Fixes eslint errors in InlineBoxShadowEditor

af5643e

Changed indent tabs to 4 spaces

f432432

Changed input textboxes to input range

b21a416

Fixed eslint errors

3419809

Merge remote-tracking branch 'origin/thimble-issue-2508' into thimble…

d1679dc

…-issue-2508

Fixed build errors

27bfa8d

Changes to bramble-extensions.json file

7bbd144

Merge branch 'master' into thimble-issue-2508

65afbaf

Removed unnecessary file - boxShadowValueTypes.json and changed Symbo…

411be2b

…l.iterator to simple counter over array

Cryzek added 6 commits March 10, 2018 11:35

Increased slider width and reduced the input range

a63b233

Separated BoxShadowLength and Color into different files

b3f04f1

Fixed imports

ec3c64f

Earlier, the BoxShadowEditor, BoxShadowLength, BoxShadowColor were importing and using the complete exports objects. Fixed by requiring only the component. eg. changed `require("BoxShadowInput")` to `require("BoxShadowInput").BoxShadowInput`;

Added inset functionality to box-shadow editor

3d1a4c5

Created BoxShadowInset that responds and provides control over `inset` property in box-shadow

Modified regex

8b22e23

Added fixes to handle cases where color property is equal to ""

b82cfb4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for thimble.mozilla.org#2508 - Quick Edit UI for box-shadow property #888

Fix for thimble.mozilla.org#2508 - Quick Edit UI for box-shadow property #888

Cryzek commented Oct 20, 2017

flukeout commented Oct 20, 2017

flukeout commented Oct 20, 2017 •

edited

Loading

humphd left a comment

humphd Oct 21, 2017

Cryzek Oct 21, 2017

Cryzek commented Oct 21, 2017

flukeout commented Nov 13, 2017

flukeout commented Nov 13, 2017

Cryzek commented Nov 14, 2017

humphd commented Dec 4, 2017

Cryzek commented Dec 5, 2017

humphd commented Dec 6, 2017

Cryzek commented Dec 19, 2017

Cryzek commented Mar 8, 2018

Cryzek commented Mar 10, 2018

		@@ -0,0 +1,268 @@
		define(function(require, exports, module) {

Fix for thimble.mozilla.org#2508 - Quick Edit UI for box-shadow property #888

Are you sure you want to change the base?

Fix for thimble.mozilla.org#2508 - Quick Edit UI for box-shadow property #888

Conversation

Cryzek commented Oct 20, 2017

flukeout commented Oct 20, 2017

flukeout commented Oct 20, 2017 • edited Loading

humphd left a comment

Choose a reason for hiding this comment

humphd Oct 21, 2017

Choose a reason for hiding this comment

Cryzek Oct 21, 2017

Choose a reason for hiding this comment

Cryzek commented Oct 21, 2017

flukeout commented Nov 13, 2017

flukeout commented Nov 13, 2017

Cryzek commented Nov 14, 2017

humphd commented Dec 4, 2017

Cryzek commented Dec 5, 2017

humphd commented Dec 6, 2017

Cryzek commented Dec 19, 2017

Cryzek commented Mar 8, 2018

Cryzek commented Mar 10, 2018

flukeout commented Oct 20, 2017 •

edited

Loading