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

prototype(tabs): create prototype tabs based on MDC web #16805

Merged
merged 1 commit into from
Sep 9, 2019

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Aug 17, 2019

Adds an implementation of the Angular Material tabs that is based on MDC web. The API is exactly the same, except for a couple of differences:

  1. The ink bar has been switched to use MDC's tab-indicator. As such the styling and the animation are slightly different.
  2. Previously MatTabLink used to be a Directive, however now it's a Component, because we need some extra markup around the content.

Note: this is not merge safe, because I had to move a few things around in the current tabs module.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 17, 2019
@crisbeto crisbeto force-pushed the mdc-tabs branch 3 times, most recently from a6aa3e5 to 176ea0d Compare August 17, 2019 12:00
@crisbeto crisbeto marked this pull request as ready for review August 17, 2019 12:17
@crisbeto crisbeto requested a review from andrewseguin as a code owner August 17, 2019 12:17
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Aug 17, 2019
@jelbourn
Copy link
Member

cc @abhiomkar FYI

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

I looked at the demo locally, seems like the label font color in dark mode is still black. Is that part of the tabs theme?

src/material-experimental/mdc-tabs/_tabs-common.scss Outdated Show resolved Hide resolved
src/material-experimental/mdc-tabs/tab-body.scss Outdated Show resolved Hide resolved
}
}

// MDC doesn't support disabled tabs so we need to improvise.
Copy link
Member

Choose a reason for hiding this comment

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

We should file this as a feature request (or bug) since aria-disabled is a supported state for role="tab"

Copy link
Member Author

Choose a reason for hiding this comment

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

My guess is that it's not there, because it's not in the spec either: https://material.io/components/tabs/#states.

@crisbeto
Copy link
Member Author

@jelbourn @mmalerba all of the feedback has been addressed.

@crisbeto crisbeto force-pushed the mdc-tabs branch 2 times, most recently from 4c3c88f to 503d3c4 Compare August 26, 2019 19:31
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 26, 2019
@crisbeto
Copy link
Member Author

Looks like I rebased this while there were build errors in master. It should be rebased once #16928 gets in.

@jelbourn
Copy link
Member

jelbourn commented Sep 4, 2019

@crisbeto when you get a chance, could you rebase this again?

@crisbeto
Copy link
Member Author

crisbeto commented Sep 4, 2019

Rebased @jelbourn.

Adds an implementation of the Angular Material tabs that is based on MDC web. The API is exactly the same, except for a couple of differences:

1. The ink bar has been switched to use MDC's `tab-indicator`. As such the styling and the animation are slightly different.
2. Previously `MatTabLink` used to be a `Directive`, however now it's a `Component`, because we need some extra markup around the content.
@jelbourn jelbourn merged commit 492fdcc into angular:master Sep 9, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants