Editable paginated instructions - #1582
Conversation
28b44dc to
e68b3c8
Compare
e68b3c8 to
78d82c9
Compare
3b21d86 to
19f8ffa
Compare
19f8ffa to
5688b11
Compare
5688b11 to
38a75a5
Compare
DNR500
left a comment
There was a problem hiding this comment.
I think this is looking good.
I’ve left a comment that more asking about the the pagination visibility/behaviour.
Separately, I was wondering how we’re confident this works when older projects are still using the “step-less” instructions format. Do we expect both formats to be live at the same time (so we should test both)?
| } | ||
| Footer={ | ||
| hasMultipleSteps ? () => <ProgressBar panelRef={panelRef} /> : undefined | ||
| hasInstructions && (hasMultipleSteps || onEditTab) |
There was a problem hiding this comment.
In the non-editable case, we still render the ProgressBar footer whenever we have instructions and the internal tabIndex happens to be 0 (I think that tabIndex defaults to 0 even though the tabs aren’t shown). I don’t know what the preferred behaviour is here but that can make the pagination UI appear when we probably only want it for the “more than 1 step” case.
Maybe we should make the footer render condition match the design rule directly (e.g. only show this pagination UI when steps.length > 1), instead of depending on the edit tab state?
There was a problem hiding this comment.
Good catch, I will look at this case
Yes they will be both used for a time, and we could do a migration in editor-api to switch to one format after this is deployed. The work I did to make sure we could handle both are in was shipped in #1580 by adding a new selector and there are some unit tests in there. I think there is coverage missing in the InstructionPanel component so I'll add another unit test there. |
Authors could only edit instructions as one big markdown string, with no way to create or delete steps. The edit tab now targets the currently-open step's markdown_content, and Add step/Remove step buttons (below the pager, always visible while editing so authors can grow past a single step) insert or remove steps relative to the current position. Removing the last step falls back to the existing empty state rather than needing special-case handling. This moves the saved shape of project.instructions from a single string to an array of step objects once any editing happens; the read side already supported this via selectInstructionSteps.
INSTRUCTIONS.md zipped project.instructions verbatim, assuming a single markdown string. Now that editing produces an array of step objects, join each step's markdown_content so downloaded projects still get a readable INSTRUCTIONS.md.
Adds spec-wc-instructions.cy.js: loads a project with existing multi-step markdown instructions (owned by the logged-in user so autosave is active), adds a step and edits it, then asserts the autosaved PUT body contains the full instructions array with the new step positioned after the one that was open — the actual contract the project saver now needs to uphold. Verified locally by running Cypress inside the docker-compose app container (installed Xvfb + GTK there for headless Electron), since the host's cached Cypress binary wasn't runnable in this environment.
Neither field is read anywhere for markdown steps (quiz is only meaningful as the separate quiz-questions object in state.instructions.quiz, and title is never read at all). New steps now only carry markdown_content.
A step added via "Add step" previously started as an empty markdown document, giving the author no cue about what to fill in. Default it to a heading placeholder instead, and update the e2e spec to assert the new step carries that default content before it's edited.
"Remove step" already removes each step (repeatedly, if needed, all the way down to none), so the header "Remove instructions" button and its confirmation modal are redundant. Removing them here; a lighter-weight confirmation modal scoped to removing a single step follows separately.
Clicking "Remove step" removed the current step immediately, with no way back. Route it through a confirmation modal instead, matching the pattern used elsewhere in the sidebar for destructive actions.
We want them to only display when the tab panel is on edit view but want them to display outside the tab panel. To do this i've made the panel a controlled component so we can manage the state and get access to it. I've made some changes to buttons.scss as previously teh secondary danger state wasn't supported.
I'm not sure why we would ever want to hide it. This is only used in the project-ui where it's set to 'display flex'
It doesn't make sense to allow the user to resize as the container prevents you from making it larger.
We think this will be clearer for educators
I'm not sure why this is here, it doesn't seem to be useful and it adds a lot of space at the bottom of the instructions panel when a footer is used. The only place that this is configured is in the projects site where it is set to to 0.
Use the background colour of the parent
We don't need two sets of instructions defined for this
When there are no tab, the tab index is set to 0 There doesn't seem to be a better way to refer to tabs by name in react-tab which would be helpful here.
38a75a5 to
2cf277e
Compare
Related to https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1681
This adds editable paginated markdown instructions
The editor already had support for paginated instructions for the project site, and editable instructions for classroom. This combines both.
instructions.mov
Note that the top (tabs) and bottom (prev/next buttons) of the panel isn't yet styled as designs. This will be done as separate issues
See commits for more.