Skip to content

Re-adding animation for drag-n-drop to reorder columns #7213

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.

Sign up for GitHub

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

Jump to bottom
Merged
KevinTCoughlin merged 7 commits into from
Nov 30, 2018
Merged

Re-adding animation for drag-n-drop to reorder columns #7213

KevinTCoughlin merged 7 commits into from
Nov 30, 2018

Conversation

@arkogupta
Copy link
Contributor

@arkogupta arkogupta commented Nov 23, 2018 •
edited
Loading

Pull request checklist

Description of changes

This adds back the animation effect on the border while dragging a column and also while dropping it. CSS3 transition property is being used here. The class names are being added programmatically and are removed after a certain interval.

Also, there is no regeneration of stylesheets on each re-render, the root cause because of which animation was removed in the first place. This has been verified as well.

Video link for the same:
https://microsoft-my.sharepoint.com/:V:/p/arkgupta/Ea749SV3KclCuqf0KPQayOQBp_xhXxFRTqZpqJW2RNGsbg?e=7VoOQp

Microsoft Reviewers: Open in CodeFlow

@arkogupta
Copy link
Contributor Author

arkogupta commented Nov 26, 2018 •
edited
Loading

The screener test is not able to capture the border while dragging, possibly because CSS3 transition has been used rather than animation. The border fades away after 0.2s

@KevinTCoughlin KevinTCoughlin self-requested a review November 26, 2018 21:30
KevinTCoughlin
KevinTCoughlin reviewed Nov 29, 2018
View reviewed changes
Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested and this does re-add the drag & drop border fade without appending stylesheets to <head> per interaction with the DetailsList, so that's good.

It seems like the transition classes can be simplified, but I have limited experience with mergeStyles & CSS3 transitions. Tagging @Vitalius1 if he has a moment to look. I don't want to block on it given the patterns already exist in the component, but if its low effort to improve we should try to.

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Nov 29, 2018

Tagged a few folks who may be interested in reviewing this.

Vitalius1
Vitalius1 reviewed Nov 29, 2018
View reviewed changes
@betrue-final-final
Copy link
Member

betrue-final-final commented Nov 29, 2018

How do I make this happen? I can't drag the headers at the moment.

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Nov 30, 2018

How do I make this happen? I can't drag the headers at the moment.

@betrue-final-final on http://fabricweb.z5.web.core.windows.net/pr/refs/pull/7213/merge/#/examples/detailslist/DragDrop perform the following:

  1. Toggle "Enable Column Reorder" (perhaps we want this set to true by default)
  2. Drag the "unfrozen" columns by their header cell
  3. Drop them between other column headers
  4. Note the fading border

Screencapture of the above

dl_drag_and_drop

@betrue-final-final
Copy link
Member

betrue-final-final commented Nov 30, 2018

Thanks Kevin, this looks good except for what looks like yellow highlighter on it.

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Nov 30, 2018

Thanks Kevin, this looks good except for what looks like yellow highlighter on it.

Apologies, that's the screen-capture software used. Will disable in the future if possible. Thank you for confirming!

KevinTCoughlin
KevinTCoughlin approved these changes Nov 30, 2018
View reviewed changes
Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

:shipit:

Change LGTM as it re-adds the animation previously supported without the mergeStyles perf concern.

@arkogupta, I made minor adjustment to the changelog note (a1433fa) and consolidated classList.remove calls into one since the remove method supports var args in 7a817aa.

@KevinTCoughlin KevinTCoughlin merged commit 71c6f5a into microsoft:master Nov 30, 2018
@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Nov 30, 2018

@arkogupta thanks for your patience and for doing the work here!

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Dec 3, 2018

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

Handy Links:

@msft-github-bot msft-github-bot mentioned this pull request Dec 3, 2018
Closed
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Dec 7, 2018
@Markionium
Merge branch 'master' into mapol/vertical-persona
19183d8 
* master: (55 commits)
  Applying package updates.
  default spellCheck=false in BasePicker (microsoft#7301)
  Separating border:none from .cell and removing from .root to avoid style override in DetailsColumn (microsoft#7211)
  Dropdown component honors ariaLabel prop (microsoft#7292)
  Add extensions for PowerShell, HEIC, 3gp, mak to FileTypeIconMap (microsoft#7299)
  Fix microsoft#7258 - Documentation is missing for button (microsoft#7302)
  Applying package updates.
  Add customizer scope to TeachingBubbleContent (microsoft#7294)
  Applying package updates.
  DevExp: Part 2 of 2 - the codebase no longer uses const enums, enabling consumers to be able to use isolatedModules (microsoft#7119)
  Add card dgl functionality (microsoft#7287)
  Only support LTS versions of Node 8 and 10 (microsoft#7290)
  add shift into delete action (microsoft#7274)
  Update CODEOWNERS
  Adding RTL support for DetailsList drag-drop to reorder columns (microsoft#7267)
  Applying package updates.
  Improve TeachingBubble usage docs (microsoft#7266)
  Lifting the resolution of default and user provided style variables to Utilities. (microsoft#7261)
  Re-adding animation for drag-n-drop to reorder columns (microsoft#7213)
  Update DetailsList Drag & Drop Example Snapshot (microsoft#7272)
  ...
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Vitalius1 Vitalius1 Vitalius1 left review comments

@KevinTCoughlin KevinTCoughlin KevinTCoughlin approved these changes

@natalieethell natalieethell Awaiting requested review from natalieethell

Assignees

@aneeshack4 aneeshack4

Labels

Component: DetailsList

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DetailsList: Re-add drag & drop border fade-out animation effect without stylesheet regen

7 participants

@arkogupta @KevinTCoughlin @betrue-final-final @msft-github-bot @natalieethell @Vitalius1 @aneeshack4