Skip to content
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.

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

Gradient outlines more fixes #1243

Merged
merged 9 commits into from
Aug 31, 2020
Merged

Conversation

fsih
Copy link
Contributor

@fsih fsih commented Aug 28, 2020

Resolves

Fixes #1224
Fixes #1239
Fixes #1230

Test branch: https://fsih.github.io/scratch-gui/gradientOutlines/

Proposed Changes

  1. Show solid transparent instead of a gradient when the stroke width is 0
  2. When you change a transparent, solid fill or stroke to one of the gradient types, it should change the color to a black-to-white gradient (instead of transparent-to-purple)
  3. Remove the "mixed" (3 circles) color from any of the swatches when switching to the rectangle or ellipse tools

Reason for Changes

  1. We want the state to be "the stroke width is 0 iff the stroke color is transparent" since it can be confusing if nothing is drawing because one or the other is true
  2. Design decision, half-transparent gradients feel conceptually higher ceiling
  3. "mixed" is not a color you can paint with

Test Coverage

Tested at a paint huddle
Tests planned later: #1216

DD Liu added 7 commits August 21, 2020 10:28
…to a transparent solid color on select, because that was causing the bug where when you switched gradient twice with trans-trans, it became black-white. Instead, leave the color as a trans-trans gradient, but treat it as if its solid when switching colors. Its starting to keep track of a lot of secret state, which seems brittle...
item.strokeColor.gradient.stops[0].color.alpha === 0 &&
item.strokeColor.gradient.stops[1].color.alpha === 0) {
// Clear the gradient if both colors are transparent
item.strokeColor = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like a function which retrieves color state from shapes should not modify those shapes' colors directly

@fsih fsih changed the title Gradient outlines2 Gradient outlines more fixes Aug 28, 2020
@fsih fsih requested a review from ericrosenbaum August 31, 2020 11:58
@fsih fsih added this to the August 2020 milestone Aug 31, 2020
Copy link

@ericrosenbaum ericrosenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed interactively with @fsih and LGTM!

@fsih
Copy link
Contributor Author

fsih commented Aug 31, 2020

Filed another bug #1248 for more tests

@fsih fsih merged commit a367d54 into scratchfoundation:develop Aug 31, 2020
@fsih fsih deleted the gradientOutlines2 branch August 31, 2020 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment