-
Notifications
You must be signed in to change notification settings - Fork 479
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
Add SelfPacedProgressTable to My PL LandingPage component #57869
Changes from all commits
af2f1ce
3797990
0fee42b
7168e26
ad9ced7
b0cf37b
13b5466
7b77ae3
98bc137
14dfbbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ const CourseRow = ({ | |
{percent_completed}% {i18n.selfPacedPlCompleted()} | ||
</BodyThreeText> | ||
{/* Progress bar */} | ||
<div className={styles.progressBar}> | ||
<div className={styles.progressBar} data-testid="progress-bar"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thought for future/followup work- it would be super cool if this had some sore of accessible readout (i.e. label) of progress! I know that isn't a focus for this page but always something to keep in mind in case it's easy to add. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about adding something like that! But it says "(percent)% Completed" right above it so I thought it would be redundant 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oo does a screen reader read that out loud? I'm not sure if this is what you're referring to, but there is a way to add an aria-label that doesn't show, but a screen reader would still read There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh it doesn't! VoiceOver on Mac only reads links and buttons. I checked this on the Teacher homepage, and it does the same thing there too (only reads links and buttons). I'll add a ticket for this in the next AccHack. Edit: made a ticket: https://codedotorg.atlassian.net/browse/A11Y-281 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay- it should be as simple as wrapping the text in a . But yes- thanks for making a ticket! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh yes okay I'm following! To be clear, folks using screen readers use the tab key to scan through interactive elements (like you mentioned- buttons, links, input fields, etc), but they can also use keys to navigate by header size (and drill down into reading the text directly if they choose). This is a super interesting case because A11y-wise, really we should be treating this div as an image with an alt text. Happy to brainstorm together at some point! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A ha! That makes sense, thanks for explaining. This is an interesting case…maybe the Let's def brainstorm on this, and thanks for bringing it up! |
||
<span | ||
className={styles.progressBarFill} | ||
style={{width: `${percent_completed}%`}} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,11 @@ | ||
import React from 'react'; | ||
import SelfPacedProgressTable from './SelfPacedProgressTable'; | ||
import {selfPacedCourseConstants} from './constants'; | ||
|
||
export default { | ||
component: SelfPacedProgressTable, | ||
}; | ||
|
||
const plCoursesStarted = [ | ||
{ | ||
name: '#', | ||
title: 'Course is in progress', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 10, | ||
finish_url: '', | ||
}, | ||
{ | ||
name: '#', | ||
title: 'Course is in progress and has unit certificates', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 45, | ||
finish_url: '#', | ||
}, | ||
{ | ||
name: '#', | ||
title: 'Course is complete', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 100, | ||
finish_url: '#', | ||
}, | ||
]; | ||
|
||
export const Basic = () => { | ||
return <SelfPacedProgressTable plCoursesStarted={plCoursesStarted} />; | ||
return <SelfPacedProgressTable plCoursesStarted={selfPacedCourseConstants} />; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
export const selfPacedCourseConstants = [ | ||
{ | ||
name: '#', | ||
title: 'Course is in progress', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 10, | ||
finish_url: '', | ||
}, | ||
{ | ||
name: '#', | ||
title: 'Course is in progress and has unit certificates', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 45, | ||
finish_url: '#', | ||
}, | ||
{ | ||
name: '#', | ||
title: 'Course is complete', | ||
current_lesson_name: 'Current lesson name', | ||
percent_completed: 100, | ||
finish_url: '#', | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,11 +10,12 @@ main.wrapper { | |
} | ||
|
||
section { | ||
padding: 4rem 1rem 0 1rem !important; | ||
padding: 4rem 0 0 !important; | ||
} | ||
|
||
.headerContainer { | ||
background-color: color.$light_gray_50; | ||
padding: 0 1rem; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did a little bit of refactoring here related to this PR since the inline padding was being applied to all the |
||
} | ||
|
||
.headerWithoutTabsContainer { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ table.selfPacedProgressTable { | |
height: 0.5rem; | ||
border-radius: 1rem; | ||
margin-block-start: 0.25rem; | ||
overflow: hidden; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That attention to detail🔥 |
||
|
||
.progressBarFill { | ||
display: block; | ||
|
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.
Added this to remove the bottom margin from the Getting Started
TwoColumnActionBlock
component used on this page for new teachers.