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(business): implement tabs #210

Merged
merged 12 commits into from
Nov 14, 2019
Merged

feat(business): implement tabs #210

merged 12 commits into from
Nov 14, 2019

Conversation

tyzess
Copy link
Contributor

@tyzess tyzess commented Nov 1, 2019

Closes #87

@tyzess tyzess added the comp: lean Issues related to @sbb-esta/angular lean design label Nov 1, 2019
@tyzess tyzess requested a review from kyubisation November 1, 2019 11:18
@tyzess tyzess self-assigned this Nov 1, 2019
Copy link
Collaborator

@kyubisation kyubisation 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 very much for your contribution 😃
Looks very good already. I've requested some changes.

@kyubisation
Copy link
Collaborator

@tyzess tyzess requested a review from kyubisation November 4, 2019 14:23
@Frenggi
Copy link

Frenggi commented Nov 5, 2019

Hi @kyubisation
The initial implementation looks ok. But the intent was to show tabs like they are known from RCP applications. Therefore the inactive tab is not as designed. Inactive != disabled. --> check attached image.
Tabs

I'm aware, that this design is a bit trickier to implement in a component than the one for tabs in public library. Therefore we might have to discuss what possibilities we have as a compromise between design and the implemented component.
The second point is the pill: there it would be great to place it inside the frame of the tab. But, I understand that you tried to reuse the component implementation from the public library. And therefore I'm not sure if it would be possible to place it inside.

@tyzess
Copy link
Contributor Author

tyzess commented Nov 5, 2019

@Frenggi Thanks for the hint. I confused inactive with disabled. Could you update https://designsystems.app.sbb.ch/en/webapps/components/tab and add a spec for disabled tabs?
Regarding the pills, they just came with the port from public to business. Until now I just ignored them.

@tyzess
Copy link
Contributor Author

tyzess commented Nov 8, 2019

@kyubisation Is the build pipeline currently broken?

@kyubisation
Copy link
Collaborator

I switched from travis-ci to github actions, however there was a problem with the scripts name, which I fixed just now. You can rebase on master and then it should work.
Sorry for the slow response on this PR. I will have a meeting on monday with @Frenggi in regard to the disabled tabs. Afterwards I'll come back to this.

@tyzess
Copy link
Contributor Author

tyzess commented Nov 12, 2019

@kyubisation Any update? We'd appreciate it if we could use the tabs in our projects soon...

@kyubisation
Copy link
Collaborator

@uzysset Please rebase the PR on top of master and force push the changes. We will merge it as it currently is and open another issue for adaptation to the new design spec.

Copy link
Collaborator

@kyubisation kyubisation left a comment

Choose a reason for hiding this comment

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

LGTM 😃
Thank you very much!

@kyubisation kyubisation merged commit dda0b43 into master Nov 14, 2019
@kyubisation kyubisation deleted the feature/tabs-business branch November 14, 2019 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: lean Issues related to @sbb-esta/angular lean design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(business): tabs styling
3 participants