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

Is there any reason to not use tabView.SelectedIndex()? #1100

Closed
Summon528 opened this issue Jun 1, 2019 · 2 comments · Fixed by #1117
Closed

Is there any reason to not use tabView.SelectedIndex()? #1100

Summon528 opened this issue Jun 1, 2019 · 2 comments · Fixed by #1117
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@Summon528
Copy link
Contributor

void App::_SetFocusedTabIndex(int tabIndex)
{
auto tab = _tabs.at(tabIndex);
_tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tab, this](){
auto tabViewItem = tab->GetTabViewItem();
_tabView.SelectedItem(tabViewItem);
});
}

Upon investigating #1074 & #1038, I was wondering if there is any reason that we are going through all the hassle of finding the correct tabViewItem from vector tabs instead of just something like

    void App::_SetFocusedTabIndex(int tabIndex)
    {
        _tabView.SelectedIndex(tabIndex);
    }

I guess the diffrence is that I use the index from _tabView.Item() instead, but does tabs and _tabView.Items() ever get out of sync?

@Summon528 Summon528 added the Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs label Jun 1, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 1, 2019
@zadjii-msft
Copy link
Member

I honestly don't know if there's a reason. Does kinda seem to me like this would be easier...

@zadjii-msft zadjii-msft added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs labels Jun 3, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 3, 2019
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Jun 3, 2019
@Summon528
Copy link
Contributor Author

I also use the terminal with this piece of code for two days or so with no issue. Guess I'll make a PR.

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 3, 2019
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Jun 4, 2019
miniksa pushed a commit that referenced this issue Jun 4, 2019
Use tabview.SelectedIndex for setting focus tab

References
  Closes #1100, Closes #1039, Closes #1074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants