Uh oh!
There was an error while loading. Please reload this page.
feat(user feedback): Adds toolbar for cropping and annotating - #15282
Conversation
| </div> | ||
| {options._experiments.annotations && ( | ||
| <div class="editor__tool-container"> | ||
| <div /> |
There was a problem hiding this comment.
Extra div for reset button in a future PR
There was a problem hiding this comment.
yes, it's so we don't have to modify the CSS each time we add a new tool (reset button and colour picker buttons)
| <PenIcon /> | ||
| </button> | ||
| </div> | ||
| <div /> |
There was a problem hiding this comment.
Extra div for colour picking in a future PR
| ></CropCorner> | ||
| </div> | ||
| )} | ||
| {(!options._experiments.annotations || isCropping) && ( |
There was a problem hiding this comment.
To be moved into the toolbar area in a future PR, so I didn't combine it with above
size-limit report 📦
|
| const [isCropping, setIsCropping] = hooks.useState(true); | ||
| const [isAnnotating, setIsAnnotating] = hooks.useState(false); |
There was a problem hiding this comment.
We might want to make these a single state where the value is more like an enum.
Right now i'm noticing spots like this:
setIsCropping(!isCropping);
setIsAnnotating(false);
which will only get more annoying if more tools get added. Not something for now, but when the next one gets added this'll be a fix.
| if (!isCropping) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
is this important to be after the clearRect() call? which rect is being cleared in this case
There was a problem hiding this comment.
it is important to be after the clearRect. It removes the white border that's used to show that it's in the cropping state. Here's waht it would look like if it was before clearRect:
ryan953
left a comment
There was a problem hiding this comment.
The Editor component is getting to be a big file & function, as we add more we should keep an eye out to split out things: it'll make it easier to reason about if different unrelated state is isolated from the rest.
For now though, time to shipit! lets go!
| </div> | ||
| {options._experiments.annotations && ( | ||
| <div class="editor__tool-container"> | ||
| <div /> |
…try#15282) - adds a toolbar for cropping and annotations - changes from inline styles to multiple class names in BEM format With annotation option:  Without annotation option (to confirm that it looks the same as before):  Closesgetsentry#15252
With annotation option:
Without annotation option (to confirm that it looks the same as before):
Closes#15252