-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/formation section #468
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
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 4729b1b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
openscript
left a comment
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.
| { | ||
| elements.map((element) => ( | ||
| <li> | ||
| <h6>{element.title}</h6> |
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 we shouldn't use h6 here, because of the following reasons:
- There is a gap in the hierarchy of chapters (e.g. no
h5is used), what is semantically incorrect. - Chapter should be used in the context of an article, but here it's inside a list.
I suggest to use a <div class="title">.
| <ol> | ||
| { | ||
| elements.map((element) => ( | ||
| <li> |
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.
For more flexibility, maybe we should add a <TimelineEntry> component and use it here via <slot>. For example I already wish to add a date or year, but I would have to change the props of the Timeline component.
| <div> | ||
| <slot name="title" /> | ||
| <slot /> | ||
| </div> |
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.
The additional slot doesn't seem to be necessary.
| <div> | |
| <slot name="title" /> | |
| <slot /> | |
| </div> | |
| <div> | |
| <slot /> | |
| </div> |
46cce2b to
db3a383
Compare
db3a383 to
985e6f3
Compare
|
@diegosteiner I've added some content, maybe you want to extend it a bit further? :) |

No description provided.