ProgressTableDetailCell - progress table refactor #38859
Conversation
maureensturgeon
force-pushed
the
maureen/LP-1672-progress-table-detail-cell
branch
from
9cc1da6
to
ed8aea1
Compare
| LevelStatus.submitted, | ||
| LevelStatus.free_play_complete, | ||
| LevelStatus.completed_assessment, | ||
| LevelStatus.readonly |
There was a problem hiding this comment.
For my education, when does the status of a level get set to LevelStatus.readonly?
There was a problem hiding this comment.
through the "manage locking" ux for assessments and surveys. it means the student can view the assessment but not edit it (as opposed to "locked", which doesn't allow student to view at all).
There was a problem hiding this comment.
Ah, I see. Is it possible for a read-only assessment to not be completed? Or do we always want to show a read-only assessment as being completed?
There was a problem hiding this comment.
that's a good question, but one for amanda. the scope of this work was intended to achieve feature parity with our existing tables, so unless things looked broken i tried not to change the design at all. but read-only is "complete" in the sense that the student can't do anything more with it, so maybe the "complete-colored" part of the box is intended to show how much of the lesson is "done" rather than "complete"?
apps/src/templates/sectionProgress/progressTables/ProgressTableDetailCell.story.jsx
Show resolved
Hide resolved
apps/test/unit/templates/sectionProgress/progressTables/ProgressTableDetailCellTest.jsx
Outdated
Show resolved
Hide resolved
| } | ||
| }; | ||
| export default class ProgressTableDetailCell extends React.Component { | ||
| static widthForLevels(levels) { |
There was a problem hiding this comment.
Where is this called from?
There was a problem hiding this comment.
ProgressTableDetailView, which needs to pass an array of explicit column widths into ProgressTableContentView. we have not yet opened a PR for either of those components though.
There was a problem hiding this comment.
Ok, I'll take your word for it. :-)
apps/src/templates/sectionProgress/progressTables/ProgressTableDetailCell.jsx
Show resolved
Hide resolved
apps/src/templates/sectionProgress/progressTables/ProgressTableDetailCell.jsx
Show resolved
Hide resolved
apps/src/templates/sectionProgress/progressTables/ProgressTableDetailCell.jsx
Show resolved
Hide resolved
…her branch and is not needed in this one.
| } | ||
| }; | ||
| export default class ProgressTableDetailCell extends React.Component { | ||
| static widthForLevels(levels) { |
There was a problem hiding this comment.
Ok, I'll take your word for it. :-)
apps/src/templates/sectionProgress/progressTables/ProgressTableDetailCell.jsx
Show resolved
Hide resolved
| (pageResult && levelProgressFromResult(pageResult)) || | ||
| levelProgressWithStatus(LevelStatus.not_tried) | ||
| ) | ||
| : null |
There was a problem hiding this comment.
Just double checking -- do we prefer null or [] (or possibly both depending on the scenario) here?
There was a problem hiding this comment.
we need it to be null because we use the presence (or absence) of that property to distinguish multi-page assessments from single-page assessments. i added this comment in #38712 in response to a similar question from Maureen, so i'm not going to re-add it here
// `pages` is used by multi-page assessments, and its presence
// (or absence) is how we distinguish those from single-page assessments
This is a rewrite of the ProgressBubbleSet a combined effort between @cforkish and @maureensturgeon, and was extracted from the full refactor work in #38309.
There are also updates to
progressTestHelpersto help with data set-up for the unit tests for theProgressTableDetailCelland for future progress table components.The unit tests were PR'd previously: #38598
Detail cell:
You can refer to this tech spec to see how this component fits into the full table architecture.
Links
Testing story
Reviewer Checklist: