Skip to content

Addon tabs component #530

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

Merged
merged 2 commits into from
Jan 22, 2020
Merged

Addon tabs component #530

merged 2 commits into from
Jan 22, 2020

Conversation

mansona
Copy link
Member

@mansona mansona commented Jan 19, 2020

This makes the "Meet Addons" section image a real component with a tabbed UI and no longer just an image

Fixes #520

@mansona mansona changed the base branch from master to redesign-rfc January 19, 2020 12:22
@ember-learn ember-learn deleted a comment from github-actions bot Jan 19, 2020
@mansona mansona requested a review from a team January 19, 2020 21:58
@github-actions
Copy link

Files that got Bigger 🚨:

File raw gzip
ember-website.js +4.44 kB +661 B
ember-website.css +69 B +27 B

Files that got Smaller 🎉:

File raw gzip
vendor.js -2.99 kB -773 B

Files that stayed the same size 🤷‍:

File raw gzip
auto-import-fastboot.js 0 B 0 B
ember-website-fastboot.js 0 B 0 B
vendor.css 0 B 0 B

Copy link
Member

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

Please put this component into the styleguide. There are other use cases for tabs.

@@ -0,0 +1,6 @@
import Component from '@ember/component';
Copy link
Member

Choose a reason for hiding this comment

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

This should definitely be in the style guide addon.

@mansona
Copy link
Member Author

mansona commented Jan 21, 2020

@MelSumner I disagree that this should be in the styleguide 🙈 We can discuss it more on Thursday but I'll summarise it quickly here too.

Since the last time that I tried to put this in the styleguide, I have been thinking a lot about this implementation and have even tried it out with a client's app. There is a lot to be desired when coming up with a truly generic implementation of Tabs that could work like this does and could be adapted for multiple places and my experimentation has not lead me to find something that is good enough to be universal yet.

The implementation in this PR is not really generic enough to move into the styleguide and definitely doesn't have a good API for people to use. I would be very much against moving this to the styleguide with a promise of "other use cases for tabs" without first having an example of one of those use cases and building something that works for both cases. If we did it now I feel very strongly that it would be a big premature optimisation 🙈

@pichfl
Copy link
Contributor

pichfl commented Jan 21, 2020

I also see this more in the website repository than in the style guide for now. We can easily extract them from here and move them to the style guide later on.

@MelSumner
Copy link
Member

We want this component for other websites, which is why I think it should be in the styleguide.

Copy link
Member

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

👍

@MelSumner MelSumner merged commit 4e6066a into redesign-rfc Jan 22, 2020
@MelSumner MelSumner deleted the addon-tabs branch January 22, 2020 17:46
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