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

fix(Dialog): fix RTL issues #1828

Merged
merged 12 commits into from
Aug 22, 2019
Merged

fix(Dialog): fix RTL issues #1828

merged 12 commits into from
Aug 22, 2019

Conversation

lucivpav
Copy link
Contributor

@lucivpav lucivpav commented Aug 20, 2019

This PR fixes the way Dialog is rendered in RTL mode.

Dialog in LTR mode:
image

Dialog in RTL mode:
image

In IE11 RTL, the question mark is placed on the left side of the sentence. This is probably not a bug, as described here.

@codecov
Copy link

codecov bot commented Aug 20, 2019

Codecov Report

Merging #1828 into master will increase coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1828      +/-   ##
==========================================
+ Coverage   69.56%   69.56%   +<.01%     
==========================================
  Files         875      875              
  Lines        7596     7606      +10     
  Branches     2219     2227       +8     
==========================================
+ Hits         5284     5291       +7     
- Misses       2304     2307       +3     
  Partials        8        8
Impacted Files Coverage Δ
...eact/src/themes/base/components/Flex/flexStyles.ts 5.26% <0%> (ø) ⬆️
.../src/themes/base/components/Dialog/dialogStyles.ts 0% <0%> (ø) ⬆️
packages/react/src/components/Dialog/Dialog.tsx 42.1% <100%> (+8.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cba28aa...7c8f58d. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 20, 2019

Codecov Report

Merging #1828 into master will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1828      +/-   ##
==========================================
- Coverage   69.49%   69.49%   -0.01%     
==========================================
  Files         875      875              
  Lines        7603     7605       +2     
  Branches     2243     2244       +1     
==========================================
+ Hits         5284     5285       +1     
- Misses       2311     2312       +1     
  Partials        8        8
Impacted Files Coverage Δ
.../src/themes/base/components/Dialog/dialogStyles.ts 0% <0%> (ø) ⬆️
packages/react/src/components/Dialog/Dialog.tsx 35.29% <100%> (+1.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57f1c1c...07425cc. Read the comment docs.

@lucivpav lucivpav changed the title fix(RTL); Dialog, Flex gap fix(RTL); Dialog Aug 20, 2019
@lucivpav lucivpav changed the title fix(RTL); Dialog fix(RTL): Dialog Aug 21, 2019
@layershifter layershifter changed the title fix(RTL): Dialog fix(Dialog): fix RTL issues Aug 21, 2019
@layershifter layershifter added the 🧰 fix Introduces fix for broken behavior. label Aug 21, 2019
CHANGELOG.md Outdated Show resolved Hide resolved
@layershifter layershifter added needs author feedback Author's opinion is asked and removed 🚀 ready for review labels Aug 21, 2019
@vercel vercel bot temporarily deployed to staging August 21, 2019 16:01 Inactive
Co-Authored-By: Oleksandr Fediashov <olfedias@microsoft.com>
@vercel vercel bot temporarily deployed to staging August 21, 2019 16:01 Inactive
@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #1828 into master will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1828      +/-   ##
=========================================
- Coverage   69.51%   69.5%   -0.01%     
=========================================
  Files         874     874              
  Lines        7603    7605       +2     
  Branches     2214    2242      +28     
=========================================
+ Hits         5285    5286       +1     
- Misses       2310    2311       +1     
  Partials        8       8
Impacted Files Coverage Δ
.../src/themes/base/components/Dialog/dialogStyles.ts 0% <0%> (ø) ⬆️
packages/react/src/components/Dialog/Dialog.tsx 35.29% <100%> (+1.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f53719f...b13ec6d. Read the comment docs.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Tested on long text, works same as on master:
image


Buttons are now looking properly:

image

vs.

image

@layershifter
Copy link
Member

Let's also add an RTL example

docs/src/examples/components/Dialog/Rtl/DialogExample.rtl.tsx

import * as React from 'react'
import { Button, Dialog } from '@stardust-ui/react'

const DialogExampleRtl: React.FC = () => (
    <Dialog
      cancelButton="إلغاء"
      confirmButton="تؤكد"
      content="هذا عن مئات لكون أفاق. تم حتى شواطيء الصفحات, من ألمانيا الأرواح تلك. ان يطول بالعمل وانتهاءً ذات, والتي الفرنسية كل حول. بـ أخذ لغزو الشرقي اوروبا. كان بـ لعملة لهيمنة وبالرغم. عن الا الحكومة البشريةً."
      header="تأكيد العمل"
      trigger={<Button content="حوار مفتوح"/>}
    />
  )

export default DialogExampleRtl

docs/src/examples/components/Dialog/Rtl/DialogExample.rtl.tsx

import getScreenerSteps from '../commonScreenerSteps'

const config: ScreenerTestsConfig = {
  steps: getScreenerSteps(),
}

export default config

docs/src/examples/components/Dialog/index.tsx

import * as React from 'react'

import Content from './Content'
+import Rtl from './Rtl'
import Types from './Types'

const DialogExamples = () => (
  <>
    <Types />
    <Content />
+    <Rtl />
  </>
)

export default DialogExamples

@lucivpav lucivpav merged commit bae248f into master Aug 22, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/dialog-rtl branch August 22, 2019 11:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. needs author feedback Author's opinion is asked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants