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: added Package manager tabs #5174

Closed
wants to merge 10 commits into from

Conversation

the-r3aper7
Copy link
Contributor

Overview

What is it?

image
image

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Use cases and why

    1. One use case
    1. Another use case

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@netlify
Copy link

netlify bot commented Sep 15, 2023

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit b22df28

@the-r3aper7 the-r3aper7 marked this pull request as draft September 15, 2023 21:03
@gioboa
Copy link
Member

gioboa commented Sep 15, 2023

This is so cool. Thanks 🚀

@zanettin zanettin added the COMP: docs Improvements or additions to documentation label Sep 15, 2023
@gioboa
Copy link
Member

gioboa commented Sep 16, 2023

Qwik UI has tabs component, I think we can use that instead of a completely fresh implementation. What do you think @shairez ?

@shairez
Copy link
Contributor

shairez commented Sep 18, 2023

great work @the-r3aper7 !

Yeah I agree @gioboa
We already fixed a lot of bugs in Qwik UI tabs (SSR, index management, etc)

You can use the headless component and just match to the styles you've written

There's even a shorthand syntax version to write a simple tabs component -

<Tabs>
      <TabPanel title="Tab 1">Content 1</TabPanel>
      <TabPanel title="Tab 2">Content 2</TabPanel>
      <TabPanel title="Tab 3">Content 3</TabPanel>
</Tabs>

@the-r3aper7 the-r3aper7 marked this pull request as ready for review September 18, 2023 17:55
@the-r3aper7
Copy link
Contributor Author

how can i resolve this conflicts?

Copy link
Contributor

@mhevery mhevery left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. The changes look great, but this PR will have to wait until #5097 is resolved.

@mhevery
Copy link
Contributor

mhevery commented Sep 18, 2023

Qwik UI has tabs component, I think we can use that instead of a completely fresh implementation. What do you think @shairez ?

But that means that #5097 will block the merging.... 🤔

@shairez
Copy link
Contributor

shairez commented Sep 19, 2023

Yep, if it was a specific problem of Qwik UI I would have suggested to merge something simpler, but we need to fix this in Qwik anyway, so I think it's better wait a bit and use Qwik UI for an accessible tabs component.

@gioboa
Copy link
Member

gioboa commented Dec 27, 2023

@the-r3aper7 the issue with Qwik UI is solved.
We can finalise the PR with the latest version of the library.
Would you like to complete it or do you need an help?

@the-r3aper7
Copy link
Contributor Author

the-r3aper7 commented Dec 28, 2023

@the-r3aper7 the issue with Qwik UI is solved.
We can finalise the PR with the latest version of the library.
Would you like to complete it or do you need an help?

i will complete the remaining work and make a PR by tomorrow currently i am travelling.

@the-r3aper7
Copy link
Contributor Author

i have created a new PR regarding this to avoid any conflicts. I am closing this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP: docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants