-
Notifications
You must be signed in to change notification settings - Fork 262
WS-1883 - Bring Article Links Block up to component standards #13560
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.
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
base: latest
Are you sure you want to change the base?
WS-1883 - Bring Article Links Block up to component standards #13560
Conversation
…ponent-standards
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
ws-nextjs-app/cypress/e2e/specialFeatures/atiAnalytics/assertions/articleLinksBlock.ts
Show resolved
Hide resolved
…ponent-standards
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
…ponent-standards
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
…ponent-standards
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
…le-link-block-up-to-component-standards
| css={styles.labelComponent} | ||
| id={ariaLabel} | ||
| data-testid="eoj-recommendations-heading" | ||
| dir={dir} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking on the reintroduction of dir, is this because Storybook/Chromatic showed the label in the incorrect position on RTL? I feel like thats more of a Storybook problem as it can sometimes set the root dir to ltr even if you set the service to something like arabic by default.
If you remove these dir attributes and test locally/preview, is the label still in the correct position?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and I couldn't fix it any other way so I reinstated it! 😓 I will remove it again though if it's just a storybook problem. I didn't realise that can sometimes be a problem, drove me bananas yesterday. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! I can take a look and see if theres anything obvious in storybook to help fix the display. It can just be a bit temperamental from memory.
|
Super minor but could the 2 fixture data files be Otherwise looks good! |
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
Resolves JIRA: WS-1883
Summary
Bringing the
ScrollablePromocomponent out of legacy and in line with our modern component standards. The component has been renamed toArticleLinksBlockas we have also updated the existing scrollable cards styling entirely and have replaced it with a stacked card format to reflect new designs given by UX (see screenshots below).LTR:
RTL:
Code changes
ScrollablePromotoArticleLinksBlock.src/app/components/ArticleLinksBlock/Promo/index.tsx.Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links