Skip to content
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

docs: add headinglevel to Nunjucks macro options #1052

Merged
merged 4 commits into from
Jan 7, 2025
Merged

Conversation

helennickols
Copy link
Contributor

No description provided.

@@ -4,6 +4,7 @@
| ---------- | ------ | -------- | --------------------------------------------------------------------------------- |
| classes | string | No | Classes to add to the timeline's container. |
| attributes | object | No | HTML attributes (for example data attributes) to add to the timeline's container. |
| headingLevel | object | No | Defaults to 2 (`<h2>`). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, in that the code renders the new line correctly but if I was being nitpicky I'd say we should add extra spaces on lines 3-6 so that the vertical lines of the table align with the new line for readability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headingLevel should be a type of numeric not object

@@ -4,6 +4,7 @@
| ---------- | ------ | -------- | --------------------------------------------------------------------------------- |
| classes | string | No | Classes to add to the timeline's container. |
| attributes | object | No | HTML attributes (for example data attributes) to add to the timeline's container. |
| headingLevel | object | No | Defaults to 2 (`<h2>`). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headingLevel should be a type of numeric not object

@helennickols
Copy link
Contributor Author

@chrispymm Sorry, don't know why I did that. Have changed and pushed.

| ---------- | ------ | -------- | --------------------------------------------------------------------------------- |
| classes | string | No | Classes to add to the timeline's container. |
| attributes | object | No | HTML attributes (for example data attributes) to add to the timeline's container. |
| headingLevel | numeric | No | Defaults to 2 (`<h2>`). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I obviously didn't read this properly last night. I thought we were usding the same description for headingLevel as from side navigation? which is:

A number for the heading level. Defaults to 4 (<h4>)

If you decided to change it that's fine, just wanted to check.

@helennickols helennickols merged commit 5d6052c into main Jan 7, 2025
7 of 19 checks passed
@helennickols helennickols deleted the timeline-macro branch January 7, 2025 09:20
@gregtyler
Copy link
Contributor

🎉 This PR is included in version 3.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants