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

Use type from state iff canRevealPassword is true #15673

Merged
merged 4 commits into from
Oct 27, 2020

Conversation

faizal3199
Copy link

Pull request checklist

Description of changes

Set the type of TextField based on below priority list:

  1. this.state.type iff this.props.canRevealPassword is set to true.
  2. Use type from props.
  3. Default to text.

Note:

$ yarn change: Command created no file.

@msft-github-bot msft-github-bot added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Oct 23, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 23, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f554d88:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 23, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
BaseButton mount 962 985 5000
Breadcrumb mount 42991 42701 5000
Checkbox mount 1695 1706 5000
CheckboxBase mount 1424 1406 5000
ChoiceGroup mount 5353 5361 5000
ComboBox mount 1022 986 1000
CommandBar mount 7874 7895 1000
ContextualMenu mount 13627 13738 1000
DefaultButton mount 1211 1206 5000
DetailsRow mount 3806 3849 5000
DetailsRowFast mount 3826 3890 5000
DetailsRowNoStyles mount 3608 3611 5000
Dialog mount 1568 1558 1000
DocumentCardTitle mount 1864 1832 1000
Dropdown mount 2687 2785 5000
FocusTrapZone mount 1752 1767 5000
FocusZone mount 1858 1911 5000
IconButton mount 1907 1893 5000
Label mount 342 349 5000
Layer mount 2091 2089 5000
Link mount 472 456 5000
MenuButton mount 1598 1624 5000
MessageBar mount 2167 2156 5000
Nav mount 3492 3461 1000
OverflowSet mount 1466 1489 5000
Panel mount 1545 1539 1000
Persona mount 900 884 1000
Pivot mount 1520 1516 1000
PrimaryButton mount 1417 1390 5000
Rating mount 8336 8331 5000
SearchBox mount 1363 1388 5000
Shimmer mount 2766 2818 5000
Slider mount 1607 1590 5000
SpinButton mount 5516 5339 5000
Spinner mount 436 435 5000
SplitButton mount 3342 3373 5000
Stack mount 526 536 5000
StackWithIntrinsicChildren mount 1652 1596 5000
StackWithTextChildren mount 5187 5001 5000
SwatchColorPicker mount 10772 10777 5000
TagPicker mount 2904 2833 5000
TeachingBubble mount 50948 51197 5000
Text mount 466 439 5000
TextField mount 1508 1539 5000
Toggle mount 867 878 5000
button mount 104 102 5000

@size-auditor
Copy link

size-auditor bot commented Oct 23, 2020

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: c8bb45d

Possible causes

  • The baseline build c8bb45d is broken
  • The Size Auditor run for the baseline build c8bb45d was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! You'll also need to check out the repo locally and run yarn change to generate a change file.

@khmakoto
Copy link
Member

Now that we're publishing beta versions of 8.0 please also port this to the master branch once it merges.

@ghost
Copy link

ghost commented Oct 24, 2020

CLA assistant check
All CLA requirements met.

@faizal3199
Copy link
Author

faizal3199 commented Oct 24, 2020

@ecraig12345 Added the change file.

@ecraig12345 ecraig12345 merged commit 11ce21b into microsoft:7.0 Oct 27, 2020
@ecraig12345
Copy link
Member

ecraig12345 commented Oct 27, 2020

@faizal3199 Thanks again for doing this! Would you mind also cherry-picking the change to master to ensure it's included in Fluent UI React 8?

@ecraig12345
Copy link
Member

First sentence of my previous comment was meant to end in ! (not a vaguely passive-aggressive ?), sorry! 😆

@faizal3199
Copy link
Author

@ecraig12345 This bug doesn't exists in master branch. Reveal password button functionality (which introduced this bug) isn't present in master branch.

@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@ecraig12345
Copy link
Member

@faizal3199 Good catch, it's definitely supposed to be in master. 🤦‍♀️ In that case I'll add the feature to master including your fix.

@faizal3199
Copy link
Author

Thank you! 😄

@ecraig12345 ecraig12345 removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants