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

feat(rtl): add span element with dir: 'auto' for the strings used in the Stardust components #704

Merged
merged 35 commits into from
Jan 22, 2019

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jan 11, 2019

This PR aims to fix the issues raised in the following #5 (comment)

In general, we need to get to a state where the text nodes are inside of elements that have dir='auto' attribute. This allows the browser to detect the text direction.

If the text is composed of multiple parts (typically a translation with parameters), each of the parts needs to be separated in element. This is however out of scope for Stardust and should be handled in the i18n layer of the application.

Requirements:

  • use Text component to render any text inside of standard Stardust components - NOTE: this is reworked in a different way, by providing rtlProps in the renderComponent function which currently contains only {dir: 'auto'} if the children or content in the component are plain string. In the future we may reworked this in a similar way the accessibility are defined, by adding slots and applying these props in different parts of the component
  • Text component renders <span dir='auto>{content} if the type of content is a string
    if user provides HTML/React nodes as an input for the component content, the user is responsible for adding dir='auto' by himself. This needs to be documented
  • revisit applying of truncateStyle in buttonStyles.ts - it needs to be applied even if the content contains or other elements - NOTE: This was broken by one of the latest change sin the factories - if there is children inside the factory it is returning just the element provided there.

-added examples in the Button and Header component
@mnajdova mnajdova changed the title feat(chore): replace string content with Text component in all Stardust components feat(Text): replace string content with Text component in all Stardust components Jan 11, 2019
@mnajdova mnajdova added the question Further information is requested, concerns that require additional thought are raised label Jan 11, 2019
@mnajdova mnajdova changed the title feat(Text): replace string content with Text component in all Stardust components feat(renderComponent): add dir: 'auto' in the components' root if the children or content is plain string Jan 14, 2019
@mnajdova mnajdova added 🚀 ready for review and removed question Further information is requested, concerns that require additional thought are raised 🚧 WIP labels Jan 14, 2019
@mnajdova
Copy link
Contributor Author

@jurokapsiar thanks for the explanation, it is fixed now. Please take a look.

# Conflicts:
#	CHANGELOG.md
#	src/components/Accordion/AccordionTitle.tsx
#	src/components/Menu/Menu.tsx
#	src/components/Menu/MenuItem.tsx
Copy link
Contributor

@kuzhelov kuzhelov left a comment

Choose a reason for hiding this comment

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

have left couple comments - could we discuss them before merge?

CHANGELOG.md Outdated Show resolved Hide resolved
@mnajdova mnajdova merged commit fdfb8bf into master Jan 22, 2019
@layershifter layershifter deleted the fix/text-component-usages branch February 19, 2019 09:14
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.

4 participants