Skip to content

fix(tabs): disabled tab link not preventing router navigation #10358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 4, 2018
Merged

fix(tabs): disabled tab link not preventing router navigation #10358

merged 1 commit into from
Oct 4, 2018

Conversation

crisbeto
Copy link
Member

Fixes users being able to navigate to a new route by clicking on a disabled mat-tab-link.

Fixes #10354.

@crisbeto crisbeto requested a review from andrewseguin as a code owner March 10, 2018 17:14
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 10, 2018
@crisbeto crisbeto force-pushed the 10354/tab-link-disabled branch from 002e0c5 to 06f7e28 Compare March 10, 2018 22:42
@crisbeto crisbeto force-pushed the 10354/tab-link-disabled branch from 06f7e28 to 7834261 Compare April 20, 2018 14:00
@crisbeto crisbeto force-pushed the 10354/tab-link-disabled branch from 7834261 to f1dc190 Compare June 11, 2018 21:17
@crisbeto
Copy link
Member Author

@andrewseguin can you take a look at this one?

@crisbeto
Copy link
Member Author

@josephperrott can you take a look at this one?

josephperrott
josephperrott reviewed Aug 7, 2018
View reviewed changes
src/lib/tabs/tab-nav-bar/tab-nav-bar.scss
// preventing the default action through JS, because we can't prevent the action reliably
// due to other directives potentially registering their events earlier. This shouldn't cause
// the user to click through, because we always have a `.mat-tab-links` behind the link.
pointer-events: none;
Copy link
Member

Choose a reason for hiding this comment

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

Will this prevent tabbing to the element and hitting enter to navigate?

Copy link
Member Author

Choose a reason for hiding this comment

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

We add tabindex="-1" when the link becomes disabled so you can't tab to it.

@josephperrott josephperrott added the target: patch This PR is targeted for the next patch release label Aug 7, 2018
@crisbeto
fix(tabs): disabled tab link not preventing router navigation
3762d26 
Fixes users being able to navigate to a new route by clicking on a disabled `mat-tab-link`.

Fixes angular#10354.
@crisbeto crisbeto force-pushed the 10354/tab-link-disabled branch from f1dc190 to 3762d26 Compare August 8, 2018 18:58
@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 9, 2018
@ngbot
Copy link

ngbot bot commented Aug 9, 2018

I see that you just added the pr: merge ready label, but the following checks are still failing:
    failure status "continuous-integration/travis-ci/pr" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

josephperrott
josephperrott approved these changes Aug 9, 2018
View reviewed changes
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@vivian-hu-zz vivian-hu-zz merged commit bf66d57 into angular:master Oct 4, 2018
roboshoes pushed a commit to roboshoes/material2 that referenced this pull request Oct 23, 2018
@crisbeto @roboshoes
fix(tabs): disabled tab link not preventing router navigation (angula…
1b9d1af 
…r#10358)

Fixes users being able to navigate to a new route by clicking on a disabled `mat-tab-link`.

Fixes angular#10354.
@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 Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@josephperrott josephperrott josephperrott approved these changes

@andrewseguin andrewseguin Awaiting requested review from andrewseguin

Assignees

@josephperrott josephperrott

@andrewseguin andrewseguin

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tab nav bar navigation to dissabled
6 participants
@crisbeto @josephperrott @jelbourn @googlebot @andrewseguin @vivian-hu-zz