Skip to content

Conversation

@ThomasMichon
Copy link
Member

Description of changes

Added a wrapper element around the drop target icons for column headers so that styles which mess with the display style for ms-Icon do not cause the target hints to all render at once. This way, the visibility of the icon is controlled independently from its style, and should now only appear when the target is active.

Focus areas to test

Dragging and dropping column headers in DetailsList.

@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 Nov 20, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 20, 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 d2200f9:

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

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Nov 20, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
BaseButton mount 971 978 5000
Breadcrumb mount 41968 42365 5000
Checkbox mount 1652 1643 5000
CheckboxBase mount 1428 1405 5000
ChoiceGroup mount 5200 5234 5000
ComboBox mount 940 967 1000
CommandBar mount 8114 7889 1000
ContextualMenu mount 13957 14248 1000
DefaultButton mount 1199 1208 5000
DetailsRow mount 3940 3899 5000
DetailsRowFast mount 3866 3895 5000
DetailsRowNoStyles mount 3700 3697 5000
Dialog mount 1571 1577 1000
DocumentCardTitle mount 1858 1861 1000
Dropdown mount 2747 2754 5000
FocusTrapZone mount 1719 1797 5000
FocusZone mount 1906 1915 5000
IconButton mount 1896 1885 5000
Label mount 339 357 5000
Layer mount 2058 2094 5000
Link mount 474 471 5000
MenuButton mount 1590 1582 5000
MessageBar mount 2088 2158 5000
Nav mount 3593 3437 1000
OverflowSet mount 1528 1464 5000
Panel mount 1505 1538 1000
Persona mount 878 859 1000
Pivot mount 1555 1544 1000
PrimaryButton mount 1363 1377 5000
Rating mount 8232 8244 5000
SearchBox mount 1347 1350 5000
Shimmer mount 2770 2776 5000
Slider mount 1595 1608 5000
SpinButton mount 5284 5274 5000
Spinner mount 427 414 5000
SplitButton mount 3372 3376 5000
Stack mount 535 526 5000
StackWithIntrinsicChildren mount 1618 1670 5000
StackWithTextChildren mount 5039 5044 5000
SwatchColorPicker mount 10887 10954 5000
TagPicker mount 2884 2997 5000
TeachingBubble mount 51394 51597 5000
Text mount 441 469 5000
TextField mount 1472 1461 5000
Toggle mount 887 882 5000
button mount 92 105 5000

@size-auditor
Copy link

size-auditor bot commented Nov 20, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react office-ui-fabric-react-DetailsList 215.672 kB 215.717 kB ExceedsBaseline     45 bytes
office-ui-fabric-react office-ui-fabric-react-ShimmeredDetailsList 226.08 kB 226.125 kB ExceedsBaseline     45 bytes

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

Baseline commit: a01b37729af7f9c168547049355dc59fadc942ba (build)

@KevinTCoughlin
Copy link
Member

CI reports snapshots need to be updated. However, npm run update-snapshots errors because there are no tests in the react-shared-contexts package.

info @fluentui/react-shared-contexts update-snapshots ❌ fail
info 🏗 Summary
info
verb @fluentui/react-stylesheets update-snapshots started - incomplete, took 0.00s
verb @fluentui/react-shared-contexts update-snapshots failed, took 18.64s
verb @fluentui/react-window-provider update-snapshots started - incomplete, took 0.00s
verb @uifabric/utilities update-snapshots started - incomplete, took 0.00s
info [Tasks Count] success: 0, skipped: 0, incomplete: 4
info ----------------------------------------------
ERR! [@fluentui/react-shared-contexts update-snapshots] ERROR DETECTED
ERR! started
ERR! hash: ba57f61859a387ac4f0ede0e9e1befcfb6064772, cache hit? false
ERR! Running C:\Users\kevin\AppData\Local\nvs\default\npm.CMD run update-snapshots
ERR! > @fluentui/react-shared-contexts@0.1.1 update-snapshots C:\Users\kevin\Work\office-ui-fabric-react\packages\react-shared-contexts
ERR! > just-scripts jest -u
ERR! [3:06:42 PM] ■ started 'jest'
ERR! [3:06:42 PM] ■ Running Jest
ERR! [3:06:42 PM] ■ C:\Users\kevin\AppData\Local\nvs\default\node.exe "C:\Users\kevin\Work\office-ui-fabric-react\node_modules\jest\bin\jest.js" --config "C:\Users\kevin\Work\office-ui-fabric-react\packages\react-shared-contexts\jest.config.js" --colors --updateSnapshot
ERR! No tests found, exiting with code 1
ERR! Run with `--passWithNoTests` to exit with code 0
ERR! In C:\Users\kevin\Work\office-ui-fabric-react\packages\react-shared-contexts
ERR!   14 files checked.
ERR!   testMatch:  - 0 matches
ERR!   testPathIgnorePatterns: \\node_modules\\ - 14 matches
ERR!   testRegex: (\\__tests__\\.*|\.(test|spec))\.(ts|tsx)$ - 0 matches
ERR! Pattern:  - 0 matches
ERR! [3:06:46 PM] x Error detected while running 'jest'
ERR! [3:06:46 PM] x ------------------------------------
ERR! [3:06:46 PM] x Error: Command failed: C:\Users\kevin\AppData\Local\nvs\default\node.exe C:\Users\kevin\Work\office-ui-fabric-react\node_modules\jest\bin\jest.js --config C:\Users\kevin\Work\office-ui-fabric-react\packages\react-shared-contexts\jest.config.js --colors --updateSnapshot

@KevinTCoughlin
Copy link
Member

CI is complaining about a missing change file:

yarn run v1.19.2
$ beachball check --changehint "Run 'yarn change' to generate a change file" -b 7.0
Checking for changes against "origin/7.0"
fetching latest from remotes "origin/7.0"
Your local repository already has change files for these packages:
  office-ui-fabric-react
Found changes in the following packages: 
  @fluentui/react-examples
ERROR: Change files are needed!

@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.

@msft-github-bot msft-github-bot merged commit 22ff232 into microsoft:7.0 Nov 25, 2020
@msft-github-bot
Copy link
Contributor

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

Handy links:

@msft-github-bot
Copy link
Contributor

🎉@fluentui/react-examples@v0.9.9 has been released which incorporates this pull request.:tada:

Handy links:

@ecraig12345 ecraig12345 removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Feb 2, 2021
@ecraig12345 ecraig12345 added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Feb 22, 2021
jdhuntington pushed a commit that referenced this pull request Feb 23, 2021
…lumn" target (#16014)

Added a wrapper element around the drop target icons for column headers so that styles which mess with the display style for `ms-Icon` do not cause the target hints to all render at once. This way, the visibility of the icon is controlled independently from its style, and should now only appear when the target is active.

Dragging and dropping column headers in `DetailsList`.
@ecraig12345 ecraig12345 removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Feb 23, 2021
jdhuntington pushed a commit that referenced this pull request Feb 23, 2021
…lumn" target (#16014)

Added a wrapper element around the drop target icons for column headers so that styles which mess with the display style for `ms-Icon` do not cause the target hints to all render at once. This way, the visibility of the icon is controlled independently from its style, and should now only appear when the target is active.

Dragging and dropping column headers in `DetailsList`.
joschect pushed a commit that referenced this pull request Feb 23, 2021
…lumn" target (#16014) (#17124)

Added a wrapper element around the drop target icons for column headers so that styles which mess with the display style for `ms-Icon` do not cause the target hints to all render at once. This way, the visibility of the icon is controlled independently from its style, and should now only appear when the target is active.

Dragging and dropping column headers in `DetailsList`.

Co-authored-by: Thomas Michon <tmichon@microsoft.com>
@msft-fluent-ui-bot
Copy link
Collaborator

🎉This issue was addressed in #17124, which has now been successfully released as @fluentui/react@v8.0.0-beta.60.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉This issue was addressed in #17124, which has now been successfully released as @fluentui/react-examples@v8.0.0-beta.7.:tada:

Handy links:

joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
…lumn" target (microsoft#16014) (microsoft#17124)

Added a wrapper element around the drop target icons for column headers so that styles which mess with the display style for `ms-Icon` do not cause the target hints to all render at once. This way, the visibility of the icon is controlled independently from its style, and should now only appear when the target is active.

Dragging and dropping column headers in `DetailsList`.

Co-authored-by: Thomas Michon <tmichon@microsoft.com>
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.

7 participants