Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(rtl): right-to-left batch 1 - FRONT-3565 #2358

Merged
merged 6 commits into from Mar 17, 2022
Merged

feat(rtl): right-to-left batch 1 - FRONT-3565 #2358

merged 6 commits into from Mar 17, 2022

Conversation

emeryro
Copy link
Contributor

@emeryro emeryro commented Mar 15, 2022

  • card
  • content-item
  • list-illustration
  • inpage-navigation
  • link
  • menu
  • spinner
  • tabs
  • tag

Notes

  • there are a few elements that will be handled in another batch (for instance description list). So display may not be fully operational yet (would need a final review when all batch are done)
  • the javascript for tabs has to be updated to fix mobile display. For now I focused on the css, we will handle the few tricky components after

@emeryro emeryro self-assigned this Mar 15, 2022
@github-actions
Copy link

github-actions bot commented Mar 15, 2022 •
edited

🚀 Deployed on https://ecl-preview-2358--europa-component-library.netlify.app

@emeryro emeryro marked this pull request as ready for review March 15, 2022 10:03
@emeryro emeryro requested a review from a team March 15, 2022 10:03
@emeryro emeryro removed their assignment Mar 15, 2022
papegaill
papegaill suggested changes Mar 16, 2022
View reviewed changes
Copy link
Contributor

@papegaill papegaill left a comment

Choose a reason for hiding this comment

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

Really nice, good result already with only little extra css!
I just noticed 2 small things:
Screenshot 2022-03-15 at 18 18 19
For the inpage, active background/border should be reversed too, but it may already be too much detail..
Screenshot 2022-03-15 at 18 20 32
for tabs, the spacing of the icon on the more button should be corrected, to see if this will be taken into account in another batch with the js fixes?

src/implementations/vanilla/components/tabs/tabs.js Outdated
@@ -190,7 +190,7 @@ export class Tabs {
newOffset = maxScroll;
}
this.list.style.transitionDuration = '0.4s';
this.list.style.transform = `translate3d(-${newOffset}px, 0px, 0px)`;
this.list.style.transform = `translate3d(${newOffset}px, 0px, 0px)`;
Copy link
Contributor

Choose a reason for hiding this comment

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

this breaks the navigation for the non-rtl tabs in mobile, the pagination is in reverse, I won't apply this change now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I did it for testing and forgot to remove... I'll update

@emeryro
Copy link
Contributor Author

emeryro commented Mar 16, 2022

I updated the inpage nav, to have something working well in both cases, and reverted the wrong js update.
For the button, it is indeed handled in another PR

@papegaill papegaill merged commit 8ca874b into v3-dev Mar 17, 2022
@papegaill papegaill deleted the FRONT-3565-rtl-batch1 branch March 17, 2022 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@papegaill papegaill papegaill requested changes

Assignees
No one assigned
Labels
tag: new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@emeryro @papegaill