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

docs(tabs): remove invalid property in documentation #1140

Merged
merged 2 commits into from
Aug 31, 2016

Conversation

devversion
Copy link
Member

  • The focusIndex has been removed, and was actually the same as the selectedIndex from the description as well.
  • Also added a quick example for the selectedIndex

Fixes #1134

* The `focusIndex` has been removed, and was actually the same as the `selectedIndex` from the description as well.
* Also added a quick example for the `selectedIndex`
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 31, 2016
@jelbourn
Copy link
Member

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 31, 2016
@marc-ferrer
Copy link

In fact, setting selectedIndex does no get you the same result as the focus.
The selectedIndex property does not set the background in the tab selector as the focus does when you click on the tab. Which was actually my concern with #1134.

Does this issue mean that the selectedIndex will now have the same behaviour as the focus, or conversely we won't be able to set the focus on the desired tab?

@devversion
Copy link
Member Author

@marc-ferrer Sure it does not. But as said in my PR description, the documentation had the same description for the selectedIndex and focusIndex.

Also I guess, the focusIndex input has been removed, and is now only for internal use, to keep track of the current focused tab label wrapper.

If we make this public as an @Input, then it doesn't even work, because the tabs wrappers are not initialized at this time.

This functionality may come back in the future, but right now it does not make any sense (also since only tabs, which are selected are currently focusable)

@marc-ferrer
Copy link

@devversion So I guess that it is not possible to manually set focus on a tab currently. And selectedIndex behaviour is not expected to change to include the focus efect.

This functionality may come back in the future, but right now it does not make any sense (also since only tabs, which are selected are currently focusable)

Then how are we suposed to manage the tabs without mouse? When you use the tab key to select a tab from a list the focus moves accross the tabs and then you select the tab you want to move into.
How will this standard behaviour be implemented when "only tabs, which are selected are currently focusable"?

Don't get me wrong, I get that focusIndex got published as a public property when it shouldn't have been this way, and does not seem easy to implement an actual focusIndex. I just got a bit confused with that sentence of yours since what does not make any sense for me, is that only selected tabs are focusable.
All I wanted was to make sure how focus and select would actually work if selectedIndex and focusIndex had the same description. I Didn't want to be rude at all.

@devversion
Copy link
Member Author

devversion commented Aug 31, 2016

@marc-ferrer I'm sorry, my sentence was probably wrong.

I'm only talking about the things I can see right now - I'm not the author of this component.

At the moment you are able to move the focus between the different tab-label-wrappers per Arrow keys, but not with the Tab key.

A few minutes ago, I made the focusIndex property public again, to test how it would look and work.

But from my findings it seems like this property was never marked as an @Input() .


Right now, this PR just makes sure that the documentation matches the current state of implementation.

@kara kara merged commit 8b10431 into angular:master Aug 31, 2016
@devversion devversion deleted the patch-2 branch August 31, 2016 18:51
@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 6, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting focusIndex won't work in md-tab-group
5 participants