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 #160

Merged
merged 53 commits into from
Aug 9, 2022
Merged

feat: tab component #160

merged 53 commits into from
Aug 9, 2022

Conversation

mehmetranas
Copy link
Contributor

No description provided.

src/baklava.ts Outdated Show resolved Hide resolved
src/components/tab/bl-tab.css Outdated Show resolved Hide resolved
src/components/tab/bl-tab.css Outdated Show resolved Hide resolved
src/components/tab/bl-tab.ts Outdated Show resolved Hide resolved
src/components/tab/bl-tab.ts Outdated Show resolved Hide resolved
src/components/tab/bl-tab.ts Outdated Show resolved Hide resolved
@leventozen leventozen self-requested a review August 8, 2022 08:44
@muratcorlu
Copy link
Contributor

@buseselvi I think PR is ready for design review. 😊 Can you have a look?

@muratcorlu
Copy link
Contributor

@mehmetranas I just noticed that title attribute of bl-tab shows browser native tooltip. Maybe we can consider to use a different name for that attribute or move it to slot (as it first designed).

image

@muratcorlu muratcorlu linked an issue Aug 8, 2022 that may be closed by this pull request
@buseselvi
Copy link
Contributor

Can we update the tab titles to "Sub Title 2 Medium" as in the design? It looks like Sub Title 2 Regular.
Can we also make the right padding of the info icon 24px? We updated the design last week, I may have missed mentioning it separately 🥲
Total tab component height should be 56px, with 54px tab part and 2px orange indicator part in Selected status. Can we check?

Except those, it looks great! 🥳

@mehmetranas
Copy link
Contributor Author

mehmetranas commented Aug 8, 2022

@mehmetranas I just noticed that title attribute of bl-tab shows browser native tooltip. Maybe we can consider to use a different name for that attribute or move it to slot (as it first designed).

image

@muratcorlu We remove the title attribute. Now user can set the title in a slot.

@mehmetranas
Copy link
Contributor Author

@buseselvi We fixed all of them. Just not sure about Sub Title 2 Medium is the right format. Because in the design it seems Sub Title 3 Medium. Did you mean Sub Title 3 Medium?

@muratcorlu
Copy link
Contributor

@mehmetranas You also need to update stories of bl-tab-panel and bl-tab-group. They also have examples that uses title attribute.

@buseselvi
Copy link
Contributor

@mehmetranas yes it should be Sub Title 3 Medium, right 🥲🙏🏻

@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@muratcorlu muratcorlu merged commit 3491805 into next Aug 9, 2022
@muratcorlu muratcorlu deleted the 134-tab-component branch August 9, 2022 09:40
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

🎉 This PR is included in version 2.0.0-beta.18 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Apr 6, 2023

🎉 This PR is included in version 2.0.0 🎉

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab Component
5 participants