Make PaintEditor component rtl aware#617
Make PaintEditor component rtl aware#617chrisgarrity merged 2 commits intoscratchfoundation:developfrom
Conversation
Fixes the color-picker and any other popover elements. Adds an RTL prop to the PaintEditor that initializes the `layout` state in redux. Any other components that need to know the layout refer to the state in redux. I debated whether the state should just be a boolean (true for RTL), or ‘rtl’, ‘ltr’. I went with the latter, but could be convinced that boolean would be better. I did not add `dir=“rtl”` to the font picker dropdown as all the names are in LTR languages. Question: Should the sliders reverse direction, and if so, is it worth doing right now when the layout of the color picker may change. Adding the rtl prop and the `dir=…` to the PaintEditorComponent fixes layout issues in the playground.
|
Oh wait, I realized I need to change GUI too |
There was a problem hiding this comment.
Ok new review, now that I've fixed GUI
The preview of the gradient doesn't match what's actually applied when an item is selected or filled with the fill bucket. I'm not sure if the fill/select tools should change (that it fills right to left when you're in a right to left language, but if you switch languages, then suddenly the colors you have selected change) or if the two colors should not change sides, and in RTL there will be the weird behavior that the main color becomes the "second" color when switching to a gradient.
fsih
left a comment
fsih
left a comment
There was a problem hiding this comment.
Looks good! You might want to hold off on merging the paint update to GUI until GUI is updated to provide the rtl prop to the paint editor
Pass RTL state to paint editor - needed for scratchfoundation/scratch-paint#617 to actually work.
2 participants
Fixes the color-picker and any other popover elements.
Resolves
Resolves #614
Proposed Changes
Adds an RTL prop to the PaintEditor that initializes the
layoutstate in redux. Any other components that need to know the layout refer to the state in redux.I debated whether the state should just be a boolean (true for RTL), or ‘rtl’, ‘ltr’. I went with the latter, but could be convinced that boolean would be better.
I did not add
dir=“rtl”to the font picker dropdown as all the names are in LTR languages.Question: Should the sliders reverse direction, and if so, is it worth doing right now when the layout of the color picker may change.
Adding the rtl prop and the
dir=…to the PaintEditorComponent fixes layout issues in the playground.Test Coverage