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

PeoplePicker - Korean Characters issue in IE11 #10432

Merged
merged 7 commits into from
Sep 18, 2019

Conversation

amyngu
Copy link
Contributor

@amyngu amyngu commented Sep 11, 2019

Pull request checklist

Description of changes

  • Remove work around for React issue for Korean IME (fixed in React 16.5)
  • Listen to onCompositionUpdate instead of onInput when composing in IME

Focus areas to test

Autofill & People Pickers

Microsoft Reviewers: Open in CodeFlow

@amyngu
Copy link
Contributor Author

amyngu commented Sep 11, 2019

@yumikohey @Adjective-Object not sure why I can't add reviewers, please take a look

@size-auditor
Copy link

size-auditor bot commented Sep 11, 2019

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react ExtendedPicker 82.92 kB 83.061 kB ExceedsBaseline     141 bytes
office-ui-fabric-react Autofill 24.622 kB 24.763 kB ExceedsBaseline     141 bytes
office-ui-fabric-react ComboBox 224.11 kB 224.251 kB ExceedsBaseline     141 bytes
office-ui-fabric-react Pickers 261.481 kB 261.622 kB ExceedsBaseline     141 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 3682e4b101a264192d36d2a7ceb59ad877661e6f (build)

Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment

Choose a reason for hiding this comment

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

@joschect please review

@joschect
Copy link
Contributor

@KevinTCoughlin yep, I'm taking a look at it now. It's taken me a sec to get my windows machine setup to test.

@joschect
Copy link
Contributor

@amyngu awesome job! Thanks for doing that.

@KevinTCoughlin
Copy link
Member

@amyngu it looks like snapshots need to be updated for CI to pass.

@msft-github-bot
Copy link
Contributor

Hello @KevinTCoughlin!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-github-bot) and give me an instruction to get started! Learn more here.

…ll.tsx

Co-Authored-By: Kevin Coughlin <706967+KevinTCoughlin@users.noreply.github.com>
@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Sep 17, 2019

Looks like another case of @uifabric/experiments not updating when yarn update-snapshots is run from root.

image

But no snapshot files churn when running the command local to the experiments directory...

~\Work\office-ui-fabric-react\packages\experiments [keco/autoFillKoreanIE11]> yarn update-snapshots                     yarn run v1.17.3
$ just-scripts jest -u
[2:56:05 PM] ■ started 'jest'
[2:56:05 PM] ■ Running Jest
[2:56:05 PM] ■ C:\Program Files\nodejs\node.exe "C:\Users\keco.REDMOND\Work\office-ui-fabric-react\node_modules\jest\bin\jest.js" --config "C:\Users\keco.REDMOND\Work\office-ui-fabric-react\packages\experiments\jest.config.js" --passWithNoTests --colors --forceExit --updateSnapshot
 PASS  src/components/Slider/Slider.test.tsx (12.599s)
 PASS  src/components/Sidebar/Sidebar.test.tsx (14.988s)
 PASS  src/components/PersonaCoin/PersonaCoin.test.tsx (15.172s)
 PASS  src/components/Button/Button.test.tsx (15.322s)
 PASS  src/components/Toggle/Toggle.state.test.tsx (15.66s)
 PASS  src/components/Button/ButtonVariants/ButtonVariants.test.tsx (15.986s)
 PASS  src/components/Button/SplitButton/SplitButton.test.tsx (16.307s)
 PASS  src/components/Button/MenuButton/MenuButton.test.tsx
 PASS  src/components/Chiclet/Chiclet.test.tsx
 PASS  src/components/Toggle/Toggle.view.test.tsx
 PASS  src/components/Persona/Vertical/VerticalPersona.test.tsx
 PASS  src/components/SelectedItemsList/SelectedItemsList.test.tsx
 PASS  src/components/Pagination/Pagination.test.tsx
Force exiting Jest

Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished?
[2:56:28 PM] ■ finished 'jest' in 23.59s
Done in 24.93s.

@KevinTCoughlin
Copy link
Member

Figured out what's required to resolve CI.

@amyngu you'll need to do a full build via yarn build at repo root. Then run yarn update-snapshots in packages/experiments since the file modified is used in both office-ui-fabric-react and @uifabric/experiments.

@msft-github-bot
Copy link
Contributor

Component Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 790 782
BaseButton (experiments) 1047 1016
DefaultButton 1133 1048
DefaultButton (experiments) 2011 1968
DetailsRow 3355 3348
DetailsRow (fast icons) 3475 3406
DetailsRow without styles 3142 3169
DocumentCardTitle with truncation 31963 33317
MenuButton 1411 1416
MenuButton (experiments) 3569 3584
PrimaryButton 1197 1204
PrimaryButton (experiments) 2078 2117
SplitButton 2891 2949
SplitButton (experiments) 7325 7073
Stack 524 537
Stack with Intrinsic children 1233 1215
Stack with Text children 4348 4426
Text 387 382
Toggle 884 877
Toggle (experiments) 2265 2364
button 64 68

@msft-github-bot msft-github-bot merged commit dc5d00e into microsoft:master Sep 18, 2019
@msft-github-bot
Copy link
Contributor

🎉@uifabric/experiments@v7.17.0 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v7.35.0 has been released which incorporates this pull request.:tada:

Handy links:

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.

PeoplePicker - Korean Characters issue in IE11.
5 participants