Use TextLink from @dsco_/link in SmallChevronLink #44759
Conversation
| ]); | ||
| storybook | ||
| .storiesOf('TeacherSectionSelector', module) | ||
| .withReduxStore() |
There was a problem hiding this comment.
this is the only actual change in this file -- i hooked these stories up to redux now that <SmallChevronLink> uses redux
There was a problem hiding this comment.
I wonder at some point we want to stop using redux for isRtl. Would it be possible to just use the html dir instead?
There was a problem hiding this comment.
yes! although it would require some refactoring to our React styles that respond to isRtl. i don't know how doable this is with radium, though -- i think we could check the HTML dir with <Style>, but that would require refactoring and introducing a new feature of radium that we don't currently use, and i'm not sure how much we want to invest in radium since it's deprecated and we should be moving away from it.
on the storybook side, i think we would just have to add the storybook-addon-rtl addon
| import LargeChevronLink from './LargeChevronLink'; | ||
| import i18n from '@cdo/locale'; | ||
|
|
||
| export default storybook => { | ||
| return storybook | ||
| .storiesOf('ChevronLinks', module) | ||
| .storiesOf('LargeChevronLink', module) |
There was a problem hiding this comment.
I separated the SmallChevronLink and LargeChevronLink stories into separate files since the components are in separate files -- each component/file should have its own storybook file
|
@madelynkasula should the link at the top of the script overview page that links back to the course overview page use this component? https://studio.code.org/s/csd3-202 |
@dmcavoy it should! but there are a couple of reasons i don't think it should be part of this change:
in general, after this migration has shipped successfully, we should definitely begin adopting TextLink (or SmallChevronLink -- i was surprised to see that we don't use it more because we use this pattern across the site) anytime we need a link with text and/or an icon |
|
note to reviewers: i'm going to bump the version for there's no functional change, just a change in the way i was polyfilling CSS for old browsers |
This is the first integration of our component library into Code.org! 🎉
Adds the TextLink component from @dsco_/link. The existing SmallChevronLink component is a specific type of TextLink that always has a chevron icon, so it seemed like a great initial consumer.
The main visual/UX changes are:
aria-labelfor the screenreader to use as an identifier (doesn't apply to this use-case, but I provided it anyways to show best practice -- it will likely be required when the component library provides an Icon component).Storybook
(I'm hovering over the first link in the image below)
Consumers
There were only 2 consumers of SmallChevronLink, so I'm outlining the before/after for both here.
/teacher_dashboard/sections/:section_id - "View all sections" link
LTR - no visual change
RTL
Before
After
/s/:script_name - "Add New Section" link
LTR - no visual change
RTL
Before
After
Links
Testing story
Adds/updates storybook entries.