Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

chore: add RTL examples for visual testing #792

Merged
merged 8 commits into from
Jan 30, 2019

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jan 29, 2019

This PR adds mechanism for adding rtl examples for visual test. The added test/examples should end with the extension .rtl.tsx, and this is all that is necessary from developer's side (See the example for the DividerExample.rtl.tsx). Screener then will maximize this example with rtl enabled and will take screenshot of that.

</SourceRender.Consumer>
</SourceRender>
<Provider theme={{ rtl: isRtlEnabled }}>
{/*<div dir={isRtlEnabled ? 'rtl' : undefined}>*/}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure whether we should add this wrapper div that will have dir='rtl' if rtl is enabled. We have that logic in the ComponentExample component.

Copy link
Member

Choose a reason for hiding this comment

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

We definitely need it.

Without

image

With

image


const DividerExampleShorthand = () => <Divider content="مثال النص" />

export default DividerExampleShorthand
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export default DividerExampleShorthand
export default DividerExampleRtl

May be name should be changed?

CHANGELOG.md Outdated
@@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixes
- Fix layout of `Accordion` panel's title @kuzhelov ([#780](https://github.com/stardust-ui/react/pull/780))

### Features
- Add mechanism for adding rtl visual tests @mnajdova ([#792](https://github.com/stardust-ui/react/pull/792))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that we should have an entry in CHANGELOG about it, there are no any benefits for user from this change

Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

I vote for adding RTL examples to docsite as well (and switch them to RTL by default)

-added rtl ecamples for the Button and Divider components
-rtl examples are in rtl mode by default
@mnajdova
Copy link
Contributor Author

@miroslavstastny thanks for the suggestion, I've added RTL examples (switch to RTL by default), and now when you maximize an example, if you were seeing the example in RTL the maximize example will also show it in rtl... Please take a look.

Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

Please also edit SECTION_ORDER in gulp-example-menu.ts to show RTL below Usage section just above Performance

docs/src/examples/components/Button/index.tsx Show resolved Hide resolved
docs/src/routes.tsx Show resolved Hide resolved
@mnajdova mnajdova merged commit d989db1 into master Jan 30, 2019
@layershifter layershifter deleted the feat/add-rtl-screenshoots-on-screener branch January 30, 2019 13:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants