Handle RTL in modals

[chrisgarrity]

Resolves

What Github issue does this resolve (please include link)?

• Resolves RTL - Modals #2759

Proposed Changes

Describe what this Pull Request does
Wraps the modal content in a <div dir=... and sets the direction based on the isRtl state. Handles all the modals that use ReactModal directly not the Modal component.

Test Coverage

Try it:
https://chrisgarrity.github.io/scratch-gui/feature/2759-modals/?locale=he

Manual Testing:

• Beta modal - button layout is rtl
• import project modal - back arrow is reversed and input field is reversed
• WebGL modal - text is RTL
• unsupported browser modal - text is RTL

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

• Chrome
• Firefox
• Safari
• Opera - gets unsupported browser modal

Windows

• Chrome
• Firefox
• Edge

Chromebook

• Chrome

iPad

• Safari

Android Tablet

• Chrome

[kchadha]

One minor comment and also a question --
As far as I can tell, the unsupported browser modal doesn't look any different than before, is that true? And I wasn't sure, but should the image in the unsupported browser modal look any different or be flipped? I don't know if we have any examples of the entire browser changing its layout when in RTL...

[chrisgarrity]

I didn't reverse the browser image because it's just a facsimile of a browser window, but I just checked on one of the tablets and the arrows are to the right of the address bar in Chrome so perhaps it should. The other parts didn't change because everything is centered.

[chrisgarrity]

@kchadha I've added the no-op mapDispatchToProps to the errorBoundary container.

[fsih]

Not a big deal but, I think both arguments to connect are optional and not passing mapDispatchToProps is very common

C:\Users\DD\Git\scratch-gui\src\components\gui\gui.jsx:
318 });
319
320: export default injectIntl(connect(
321: mapStateToProps
322: )(GUIComponent));
323

C:\Users\DD\Git\scratch-gui\src\components\stage-header\stage-header.jsx:
180 };
181
182: export default injectIntl(connect(
183: mapStateToProps
184: )(StageHeaderComponent));
185

C:\Users\DD\Git\scratch-gui\src\containers\custom-procedures.jsx:
191 });
192
193: export default connect(
194: mapStateToProps
195: )(CustomProcedures);
196

C:\Users\DD\Git\scratch-gui\src\containers\modal.jsx:
58 });
59
60: export default connect(
61: mapStateToProps
62: )(Modal);
63

C:\Users\DD\Git\scratch-gui\src\containers\paint-editor-wrapper.jsx:
79 };
80
81: export default connect(
82: mapStateToProps
83: )(PaintEditorWrapper);
84

C:\Users\DD\Git\scratch-gui\src\containers\sound-editor.jsx:
226 };
227
228: export default connect(
229: mapStateToProps
230: )(SoundEditor);

[kchadha]

@fsih then are we wrong about what creates this warning? Because it wasn't always there...

[kchadha]

LGTM

[fsih]

@kchadha That happens because we have a tendency to use "...props" to pass props to components and then accidentally leave too many props in there. Div isn't a react component, so it doesn't know what to do with dispatch. This change gets rid of the warning for me: eb0383b