Mixed Appearance panel shows controls for valid layers in selection #3662

DivyaPrabhakar · 2016-02-03T21:41:03Z

Fixes issues #3777, #3756, #3615, #3147, and #2457

Before:

After: I have made all disabled inputs use the specified CSS color for each stop @underlines-disabled. In the examples below, blend mode and opacity are enabled and the rest of the type settings are disabled, just to give an idea of the different states relative to one another. @placegraphichere thoughts?

Issue #3756

Opacity and Blend Mode Alignment
Before:

After:

Color Picker Inputs Alignment
Before:

After:

Radius Input Alignment:
Before:

After:

Issue #3615
Before:

After:

Issue #3147
Before:

After:

Issue #2457 @placegraphichere Can I get your opinion about this too?

Before:

After:

DivyaPrabhakar · 2016-02-03T21:41:37Z

@Volfied or @chadrolfs Can you make sure the now activated fields are the correct ones? Is there anything that should not be active? One thing I saw on master also is when two text layers are selected Mixed Font Styles is active but there are no options to choose from. Is that correct behavior?

ktaki · 2016-02-03T22:50:52Z

Hi @DivyaPrabhakar Let me chime in.

Found an issue. Create a rectangle and a text layer. Change the color of the rectangle. Select both layers. Now the color of rectangle in the panel is wrong. Color picker is not available when clicking on the panel for changing the color of the rect.

When two text layers are selected, the current behavior looks right though more testing is needed.

chadrolfs · 2016-02-03T23:09:33Z

A couple more things (leveraging Taki's screenshots above):

When mixed state includes a shape and text, you can change the color of the text but not the shape (I see Take mentioned this above)
In this mixed state ^^, blend and opacity are out of alignment
There's no way that I can get the panel in to full view (without scrolling). Collapsing Effects and Export collapse up

DivyaPrabhakar · 2016-02-05T21:24:02Z

@ktaki and @chadrolfs

1.) The appearance panel does not scroll anymore in the mixed state

2.) The blend modes and opacity are now aligned

3.) The color picker for vector shapes is fixed

iwehrman · 2016-02-06T23:45:54Z

src/js/jsx/sections/style/VectorFill.jsx

@@ -181,44 +181,52 @@ define(function (require, exports, module) {
                    document={this.props.document}
                    layers={this.state.layers}
                    fill={this.state.fill} />);
+
+            if (this.props.document.layers.selected) {


This is always truthy. Maybe you want !this.props.document.layers.selected.isEmpty()?

iwehrman · 2016-02-06T23:55:28Z

It may well be inconvenient to implement this with our current component structure, but I definitely do not think showing a fill color swatch row AND a type color swatch row is the correct design when some type layers and some vector layers are selected. There should just be one swatch/blend-mode row, and the behavior should be intuitively uniform: the swatch is used to change the type AND vector colors simultaneously, as appropriate for the given selected layer. Why should the user care that these two swatches are wired up to different underlying actions? Instead it should be possible to change all the colors in a single operation.

DivyaPrabhakar · 2016-02-07T19:24:58Z

I think @iwehrman is right. Initially I was thinking that the current approach made sense because the user cases for choosing multiple layers at a time (ranked by priority) is 1.) To make a property change to all selected layers 2.) To move the selected layers on the canvas.

In the case that the user selects the layers to move them around, my initial hesitation was that we can't assume they want to apply a change to a property (like fill) to all the layers selected at once, so we need to provide the separated text color and vector fill options. But in that case, we already know the primary objective for the user is to change the placement of the layers, not change a property so it doesn't matter if we provide one or two different fill options anyway. Another thing I saw is that when the vector fill and text color are enabled, the stroke swatch is below the text. I think it should be paired with the vector fill swatch.

I will create a new branch off of this one and implement the one swatch approach for us to look at and decide.

chadrolfs · 2016-02-08T17:51:06Z

sounds good @DivyaPrabhakar, let @ktaki and I know when the new branch is ready and we'll give it a once over.

ktaki · 2016-03-04T17:57:25Z

@DivyaPrabhakar Toggle Fill seems not functional.

ktaki · 2016-03-04T19:28:59Z

The Sampler is broken in this branch. Make sure this isn't a side-effect of your change.

chadrolfs · 2016-03-04T19:40:09Z

Comments on new branch (mixed-appearance-refactoring-2):

Hitting Tab key after editing Typeface field in mixed selection state results in an error:

chadrolfs · 2016-03-04T19:46:22Z

rotating a mixed selection changes the attributes but shouldn't

DivyaPrabhakar · 2016-03-08T20:20:52Z

@ktaki and @chadrolfs -- I have updated the PR description with all the bugs that this PR should fix -- mainly UI fixes. Also the problems that you both identified before should @ktaki and @chadrolfs -- The problems that you both identified before should be fixed:

1.) Color swatch keeps the fill when the selections are rotated

2.) Sampler tool works

3.) Hitting Tab key after editing Typeface field in mixed selection state results in an error:Hitting Tab key after editing Typeface field in mixed selection state results in no error now.

chadrolfs · 2016-03-08T21:52:38Z

color picker is blinks mixed:

chadrolfs · 2016-03-08T21:56:09Z

interplay between the opacity in the color picker and the panel opacity is funky with a mixed selection that includes a pixel layer.

chadrolfs · 2016-03-08T21:57:26Z

changing the tracking fails:
Action type.setTracking failed: Error: Photoshop returned an error when playing command number: 1 Photoshop code: -25920 Photoshop message: error: -25920 message: The command “Set” is not currently available.

chadrolfs · 2016-03-08T21:58:42Z

same thing with Alignment:
Action type.setAlignment failed: Error: Photoshop returned an error when playing command number: 1 Photoshop code: -25920 Photoshop message: error: -25920 message: The command “Set” is not currently available.

ktaki · 2016-03-08T22:53:16Z

The issues with manipulating alpha value could be caused by the issue reported in #3786 in which the alpha value for the text layers is treated differently from that of shape layers.

ktaki · 2016-03-09T00:14:04Z

I did not find any other new issues.

DivyaPrabhakar · 2016-03-10T07:25:24Z

src/js/jsx/sections/style/AppearancePanelSections.jsx

+                    if (characterStyles && !characterStyles.isEmpty()) {
+                        var textColors = collection.pluck(characterStyles, "color");
+                        var vectorColors = this.state.vectorFill.colors;
+                        var temp = textColors.concat(vectorColors);


@iwehrman Can you help me here? The problem that happens is that textColors is in the same format as vectorColors but apparently the r, g, b, a, values of textColors is not rounded to the nearest integer and vectorColors are. Should I just call Math.round() on each of these values or am I missing something if I do that? Just not sure because we clearly handle text colors and vector fills differently so I wanted to double check. @pineapplespatula You worked with type right? So maybe you know better?

The end goal of this is getting all the vector fills and all the text colors, and putting them into one final data structure that is of the same type as textColors.

DivyaPrabhakar · 2016-03-10T07:30:57Z

@chadrolfs I fixed the errors that were occurring on alignment and tracking and I fixed part of the opacity issue -- before if you used the slider, only text and vector layers would change opacity, but now all layers change opacity. Also the user can change opacity via the opacity input field i.e. type opacity = 30 and all the layers will adjust accordingly.

2 known problems still exist -- 1.) When the opacity slider is adjusted, the opacity input field does not reflect this change. This is because of the open bug @ktaki mentioned above, and we have decided not to fix that bug. 2.) The color swatch overlay goes to mixed state when the user rotates the layers. I know why this is happening and was working on the fix but I need a little help from @iwehrman or @pineapplespatula, and I have commented on that line and will wait for their feedback.

The rest of the issues both you and @ktaki found should have been addressed. I will let you know right when a decision on problem no. 2 is made. Sorry for the delay on this, it took me some time to narrow down the problem 2.

…e not uniform kind

DivyaPrabhakar · 2016-03-10T19:34:07Z

@chadrolfs The rotation error should be fixed now!

chadrolfs · 2016-03-10T22:07:21Z

OK, I think this'll do it! Thanks for all your hard work on this. Ok to merge!

DivyaPrabhakar · 2016-03-10T23:40:29Z

src/js/models/color.js

-            "r": rgb.red,
-            "g": green,
-            "b": rgb.blue,
+            "r": mathjs.round(rgb.red),


FYI: This is because colors for type layers were returning as floating points and vector layers colors were integers. Created a problem when we called .uniformValue() on a group of type and vector colors, even though they were the same.

baaygun · 2016-03-10T23:40:52Z

Looks good to me, since it's also heavily tested, I saw no blaring errors.

…panels Mixed Appearance panel shows controls for valid layers in selection

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from 8cc5492 to d024438 Compare February 5, 2016 21:12

iwehrman reviewed Feb 6, 2016
View reviewed changes

iwehrman assigned DivyaPrabhakar Feb 6, 2016

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from d024438 to 3866298 Compare February 7, 2016 19:39

chadrolfs mentioned this pull request Feb 16, 2016

Appearance panel should not be disabled in mixed states #3147

Closed

This was referenced Mar 3, 2016

Disabled TextInput and Datalist underlines are different colors #3777

Open

Opacity/Alpha/Rotation/Radius inputs are misaligned #3756

Closed

Appearance and Effects panels should remain even when no layers are selected #2457

Closed

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from 3866298 to ea0aec4 Compare March 4, 2016 19:44

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from ea0aec4 to 597ac28 Compare March 8, 2016 20:12

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from 597ac28 to 29ae958 Compare March 8, 2016 20:25

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from 29ae958 to 021cb79 Compare March 10, 2016 07:19

DivyaPrabhakar reviewed Mar 10, 2016
View reviewed changes

DivyaPrabhakar force-pushed the divya/mixed-appearance-panels branch from 021cb79 to 5dbb862 Compare March 10, 2016 19:33

Correct inputs are active in Appearance panel when selected layers ar…

5dbb862

…e not uniform kind

DivyaPrabhakar removed their assignment Mar 10, 2016

baaygun self-assigned this Mar 10, 2016

DivyaPrabhakar reviewed Mar 10, 2016
View reviewed changes

baaygun pushed a commit that referenced this pull request Mar 10, 2016

Merge pull request #3662 from adobe-photoshop/divya/mixed-appearance-…

c82068e

…panels Mixed Appearance panel shows controls for valid layers in selection

baaygun merged commit c82068e into master Mar 10, 2016

baaygun deleted the divya/mixed-appearance-panels branch March 10, 2016 23:41

ktaki mentioned this pull request Mar 11, 2016

Each click on color picker causes re-initializing of history in mixed appearance panel #3801

Closed

iwehrman mentioned this pull request May 11, 2016

Removed extra div around rotation tool icon #3827

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixed Appearance panel shows controls for valid layers in selection #3662

Mixed Appearance panel shows controls for valid layers in selection #3662

DivyaPrabhakar commented Feb 3, 2016

DivyaPrabhakar commented Feb 3, 2016

ktaki commented Feb 3, 2016

chadrolfs commented Feb 3, 2016

DivyaPrabhakar commented Feb 5, 2016

iwehrman Feb 6, 2016

iwehrman commented Feb 6, 2016

DivyaPrabhakar commented Feb 7, 2016

chadrolfs commented Feb 8, 2016

ktaki commented Mar 4, 2016

ktaki commented Mar 4, 2016

chadrolfs commented Mar 4, 2016

chadrolfs commented Mar 4, 2016

DivyaPrabhakar commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

ktaki commented Mar 8, 2016

ktaki commented Mar 9, 2016

DivyaPrabhakar Mar 10, 2016

DivyaPrabhakar commented Mar 10, 2016

DivyaPrabhakar commented Mar 10, 2016

chadrolfs commented Mar 10, 2016

DivyaPrabhakar Mar 10, 2016

baaygun commented Mar 10, 2016

Mixed Appearance panel shows controls for valid layers in selection #3662

Mixed Appearance panel shows controls for valid layers in selection #3662

Conversation

DivyaPrabhakar commented Feb 3, 2016

DivyaPrabhakar commented Feb 3, 2016

ktaki commented Feb 3, 2016

chadrolfs commented Feb 3, 2016

DivyaPrabhakar commented Feb 5, 2016

iwehrman Feb 6, 2016

Choose a reason for hiding this comment

iwehrman commented Feb 6, 2016

DivyaPrabhakar commented Feb 7, 2016

chadrolfs commented Feb 8, 2016

ktaki commented Mar 4, 2016

ktaki commented Mar 4, 2016

chadrolfs commented Mar 4, 2016

chadrolfs commented Mar 4, 2016

DivyaPrabhakar commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

chadrolfs commented Mar 8, 2016

ktaki commented Mar 8, 2016

ktaki commented Mar 9, 2016

DivyaPrabhakar Mar 10, 2016

Choose a reason for hiding this comment

DivyaPrabhakar commented Mar 10, 2016

DivyaPrabhakar commented Mar 10, 2016

chadrolfs commented Mar 10, 2016

DivyaPrabhakar Mar 10, 2016

Choose a reason for hiding this comment

baaygun commented Mar 10, 2016