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

Feat tab component #84

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Feat tab component #84

wants to merge 5 commits into from

Conversation

goulinkh
Copy link
Contributor

@goulinkh goulinkh commented Dec 16, 2024

Done

  • Add Tabs and Tab component
image

Open Questions

  1. With such components composed of other components (Tabs + Tab) do we want to document them separately, or do we treat them as one "Tabs" component?
  2. What kind of approach would be better for composing components?
    • one root component, passing all data through props (as in the PR)
    • composing via JSX (like MUI), where <Tab> components are passed as children of <Tabs>
    • currently we decided that for tabs the props approach is simpler to implement
  3. How much functionality/state we want the component to handle, and how much do we leave to consumers to add for flexibility?
    • currently Tabs component is passed a selected tab and onChange handler, it does handle keyboard navigation support, but doesn't have internal state (state and panel switching is to be done by parent)
    • we can also implement this parent "tabs panel" component separately and expose it from DS
  4. Styling of tabs is quite canonical-oriented at the moment (bottom highlight bars on hover/active state) - how much flexibility of customisation we want users to have? it may be quite hard to make all aspects of component design to be configurable (a lot of variables needed for many different aspects of the component)

@bartaz bartaz marked this pull request as draft December 16, 2024 13:33
@advl
Copy link
Contributor

advl commented Dec 18, 2024

IMHO

  1. Only one component, Tabs should be consumed by end users. This doesn't mean we can't use internally a subcomponent Tab - yet, externally, I don't see a clear benefit of making both accessible (is there a case where Tabs are use without Tab, or with additional intermediary elements ?).
  2. See answer to 1
  3. The idea would be that state is stored only and exclusively on the url. This would ensure proper initialization of page load, as well as an easier way to reference the current content. But for this, we would need first to agree on a routing strategy, which falls outside of the scope of this PR. I believe @jmuzina is already investigating this. Additionally, this invites us to consider how to deal with routing primitives in the context of this storybook. Is it acceptable that our core storybook has an explicit routing dependency ? Would we like instead of rely on vanilla html5 history api (and in this case, how to deal with isomorphic rendering) ?
    If state is defined in the url, then we could define intrisincly the rendering behaviour at the singular Tab level, with a custom conditional if (currentUrl === tab.url) return Component
  4. Can we make most of the canonical opinions here a variable ? One thing we should keep assuming tough, is a requirement for baseline alignment.

@advl
Copy link
Contributor

advl commented Dec 18, 2024

Also, would you have any thoughts on using the html5 detail/summary tags ?

@bartaz
Copy link
Member

bartaz commented Dec 19, 2024

Also, would you have any thoughts on using the html5 detail/summary tags ?

Details/summary seems more appropriate for Accordion than Tabs I think. Especially if we want to keep an option to use tabs as a navigation (with links) instead of inline panel switcher. So having only the tab strip, without the panels themselves would not be possible with details element.

@bartaz
Copy link
Member

bartaz commented Dec 19, 2024

  1. Only one component, Tabs should be consumed by end users. This doesn't mean we can't use internally a subcomponent Tab - yet, externally, I don't see a clear benefit of making both accessible (is there a case where Tabs are use without Tab, or with additional intermediary elements ?).
  2. See answer to 1

Yes, I do agree we should treat Tab as an internal component (likely hiding it in a subfolder of Tabs).

  1. Can we make most of the canonical opinions here a variable ? One thing we should keep assuming tough, is a requirement for baseline alignment.

We can, and they are variable (for example the width and color of the thicker bottom border - so it can be changed or made invisible). But there is still a question of how much more options we want to give, that we don't use ourselves: do we want to allow other borders to be visible? border radius - just on top or all around?

There may be many ways people may want tabs to look that we don't consider yet (based on our designs).

@bartaz
Copy link
Member

bartaz commented Dec 19, 2024

Next step (based on comments above): Move Tab component inside the Tabs component folder and only expose Tabs component to be consumed from the design system.

Future (not in this PR):

  • managing state via URL
  • additional styling options, responsiveness, etc

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.

3 participants