Skip to content

Conversation

@lamxdoan
Copy link
Contributor

@lamxdoan lamxdoan commented Feb 25, 2020

Fix issue 1 in #2784

Microsoft Reviewers: Open in CodeFlow

@lamxdoan lamxdoan requested a review from a team as a code owner February 25, 2020 19:37
@ghost ghost added the vnext label Feb 25, 2020
@kmelmon
Copy link
Contributor

kmelmon commented Feb 27, 2020

int32_t m_tabIndex = std::numeric_limitsstd::int32_t::max();

Should this be initialized to -1?


Refers to: vnext/ReactUWP/Views/ViewViewManager.cpp:215 in 8fd8adc. [](commit_id = 8fd8adc, deletion_comment = False)

Copy link
Contributor

@kmelmon kmelmon left a comment

Choose a reason for hiding this comment

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

:shipit:

@lamxdoan
Copy link
Contributor Author

int32_t m_tabIndex = std::numeric_limitsstd::int32_t::max();

Yeah I think this is a more reasonable default than max int


In reply to: 592181044 [](ancestors = 592181044)


Refers to: vnext/ReactUWP/Views/ViewViewManager.cpp:215 in 8fd8adc. [](commit_id = 8fd8adc, deletion_comment = False)

@kmelmon kmelmon merged commit dfc57fc into microsoft:master Feb 27, 2020
lamxdoan added a commit to lamxdoan/react-native-windows that referenced this pull request Feb 27, 2020
* Set IsTabStop to false when tabIndex is negative

* Change files

* remove unneeded static_cast

* whoops actually did need the static_cast

* clearValue and set -1 as default
kmelmon pushed a commit that referenced this pull request Feb 28, 2020
* TextBox should have a default tabIndex of 0 (#4204)

* TextBox should have a default tabIndex of 0

* Change files

* Set IsTabStop to false when tabIndex is negative (#4180)

* Set IsTabStop to false when tabIndex is negative

* Change files

* remove unneeded static_cast

* whoops actually did need the static_cast

* clearValue and set -1 as default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants