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

CoreComponents: Add Levels for "headers" #5812

Closed

Conversation

krewast
Copy link
Contributor

@krewast krewast commented May 16, 2024

Using the "headers" function of the CoreComponents always creates h1 headers. That's a bit limiting.

This pull request introduces the new attribute "level" for the header function which is used to determine what heading level should be shown. There are officially 6 levels, the default is always h1 (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements).

Here is how it looks:

header-levels

And here is the code to check it out yourself:

<.header level="1" class="mt-[1em]">
  Header Level 1 (text-3xl)
</.header>

<.header level="2" class="mt-[1em]">
  Header Level 2 (text-2xl)
</.header>

<.header level="3" class="mt-[1em]">
  Header Level 3 (text-xl)
</.header>

<.header level="4" class="mt-[1em]">
  Header Level 4 (text-lg)
</.header>

<.header level="5" class="mt-[1em]">
  Header Level 5 (text-md)
</.header>

<.header level="6" class="mt-[1em]">
  Header Level 6 (text-sm)
</.header>

At the momemt, running mix test results in serveral warnings that all look like this:

.warning: assign @Level not available in EEx template. Please ensure all assigns are given as options.

It would be great if someone with more Elixir/Phoenix knowledge could have a look at this, because I don't really understand the issue. The attribute "level" has been defined, the type info and default value have been added. Accessing it via "@Level" doesn't differ from accessing @Class, for example. A pointer into the right direction would be helpful, I'm of course willing to add the necessary changes to fix these warnings.

@chrismccord
Copy link
Member

Something like this is a nice addition to your own components, but the core components are supposed to be a starting point and learning tool, so I don't think we need to ship this with every app. Thanks!

@krewast
Copy link
Contributor Author

krewast commented May 16, 2024

Alright, I get it, thanks for the feedback.

I just thought it would be cool to have as a default, because creating headings with different levels is such a standard thing to do. And that probably results in many reimplementations of this (or similar) logic where it could be solved once.

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

Successfully merging this pull request may close these issues.

None yet

2 participants