-
Notifications
You must be signed in to change notification settings - Fork 9
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
revising data model for EDU Lesson procedures #579
Conversation
@kimorr @otalutz Do you think this will be an acceptable limitation? Will most steps start with text? Headings at levels larger than 4 look pretty funky due to the baselines of the number and the heading being far apart, but most likely and heading within one of these steps should be an H4, since "Procedures" is an H2 and procedure section headings would be H3. |
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.
This is working pretty well for me! Just a couple needed changes in the stories, I think.
<!-- <LayoutHelper | ||
v-if="procedureSteps" | ||
indent="col-3" | ||
class="lg:mb-8 mb-5" | ||
> | ||
<BaseHeading level="h3"> | ||
{{ 'Section ' + (Number(index) + 1) }} | ||
</BaseHeading> | ||
</LayoutHelper> | ||
</LayoutHelper> --> |
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.
Did you mean to leave this in but commented?
// &::marker { | ||
// content: counter(step) '. '; | ||
// } |
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.
Did you want to save this commented block and the ones further down?
{ | ||
blocks: BlockStreamfieldMinimalData.body | ||
} | ||
] | ||
}, |
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.
I think the two procedures
items below here should probably be removed? They don't appear to be causing a problem, but they don't do anything, either.
packages/vue/src/templates/edu/PageEduLesson/PageEduLesson.stories.js
Outdated
Show resolved
Hide resolved
…ries.js Co-authored-by: Scott Cranfill <scott.cranfill@jpl.nasa.gov>
Made the changes! Forgot to do a lot of cleanup before opening the PR 🤦 thanks for the review! |
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.
LGTM! We can revisit if my question for Kim and Ota necessitates it.
Checklist
Description
Supports https://github.com/nasa-jpl/www/issues/413
Caveats
Instructions to test
make vue-storybook
Tested in the following environments/browsers:
Operating System
Browser