feat(sidebar): Allow to customize the Sidebar component with 'as'

[mvaled]

Description

In some cases, the sidebar is the main navigation of an app and setting using

    <Sidebar as="nav">
       ...
    </Sidebar>

would be more appropriate.

Type of change

• New feature (non-breaking change which adds functionality)

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (7de751e) 99.43% compared to head (b00d584) 99.43%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #703   +/-   ##
=======================================
  Coverage   99.43%   99.43%           
=======================================
  Files         131      131           
  Lines        6502     6504    +2     
  Branches      488      488           
=======================================
+ Hits         6465     6467    +2     
  Misses         37       37           

Impacted Files	Coverage Δ
src/lib/components/Sidebar/Sidebar.tsx	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rluders]

Hey, @mvaled... I got your point here.

I'm wondering if we don't want to keep the Sidebar as aside, 'cause it is a sidebar component, and move the component mutation to the Sidebar.Items or Sidebar.ItemsGroup, 'cause this component is the one that actually can hold the navigation item.

IMHO, a Sidebar can probably handle different components that are not actually related to the main site/app navigation. WDYT?

[mvaled]

@rluders

I'd rather have the sidebar completely as the nav, because the main purpose of (some) sidebars is the navigation. Non-links elements are almost always required; and they are not forbidden.

The first example of <nav> in MDN contains a tree of elements ol > li > a. (It even state that the links could be provide in prose 🤓 .)

On the other hand, an aside is a semantically anything (not just links and doesn't require items, groups, etc) that is indirectly related to the main content.

To be honest, I was somewhat surprised to see an aside as the default choice for a sidebar, for which the main purpose seems to be navigation (Sidebar.Item being mostly a link). But I didn't want the PR to be a breaking change.

The app I'm doing has a vertical navigation as a sidebar (like Sentry's navbar):

Maybe I could use Navbar instead, but that requires more work because of the default horizontal styling.

[rluders]

Well, @mvaled... it seems that you are correct. I wasn't aware of it, thanks for the explanation. So, in order to be more semantic-correct we should even avoid using the aside for the sidebar at all. Correct? So, in this case, my suggestion is to replace line#42 to use nav as default and allow people to use some other tags like div, section, or aside as they prefer.

WDYT? Does it make sense?

I found this one interesting as well: https://www.w3.org/TR/html5-author/the-aside-element.html

[mvaled]

I'll include the 'nav' as the default. Should I also include a (soft) breaking-change in the changelog. Or do you prefer to collect changelogs at the end of the release?

[rluders]

@mvaled could you check the unit tests, please? :)

[tulup-conner]

I think you can argue either way on this semantically quest easily. Sidebars often include navigation but aren't limited to it. Example: https://flowbite.com/react-admin-dashboard-pro/preview/

I do agree that nav is more specific, though, when navigation is all the box is used for. And I can't speak for the users of the library, but I do imagine you're right that that's usually the case.

And thank you for contributing, once the unit tests are ready we can merge :D

[mvaled]

@rluders The tests pass now. It was the change in the aria-label, which was reverted due to the suggestion of @tulup-conner.

[rluders]

Thank you! Let's get it merged.