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.

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

Progress: smallChevronLink component used on section progress #21797

Merged
merged 1 commit into from Apr 11, 2018

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Apr 11, 2018

Spec item: Add a link for “View course” to the top right that takes the user to the corresponding unit overview page.

screen shot 2018-04-11 at 10 43 00 am

Changes in this PR:

1.) A SmallChevronLink component, we use these teal links all over the place and componentizing it might make it more re-usable in the future.
A few places we could re-use this:
- CourseCards on the signed in homepages
- ContentContainer
- top navigation links in teacher dashboard, such as "view all sections"

2.) A Story for SmallChevronLink that also includes its sibling componentLargeChevronLink

  • combine large chevron and small chevron components - they are basically the same...
  • include RTL versions in Storybook

3.) Use SmallChevronLink on progress tables

Still to do:

  • link actually goes where it should
  • Should say “View unit” if the script is actually a unit inside a larger course, e.g. /s/csp1 inside /course/csp)

@Erin007 Erin007 requested a review from caleybrock April 11, 2018 18:53
Copy link
Contributor

@caleybrock caleybrock left a comment

Choose a reason for hiding this comment

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

This looks awesome. Big fan of the reusability. Code and screenshots look great. 💯

@@ -105,6 +114,9 @@ class SectionProgress extends Component {
scriptData={scriptData}
studentLevelProgress={studentLevelProgress}
/>
<SummaryViewLegend
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this got changed in the other PR too, intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My guess is it floated around when I was fixing a merge conflict. Since we want the legend at the bottom of the view, I moved it.

@Erin007 Erin007 merged commit 3fc7b42 into staging Apr 11, 2018
@Erin007 Erin007 deleted the view-course-link-shared branch April 11, 2018 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants