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.

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

Adding RTL support for DetailsList drag-drop to reorder columns #7267

Merged
merged 3 commits into from
Dec 3, 2018
Merged

Adding RTL support for DetailsList drag-drop to reorder columns #7267

merged 3 commits into from
Dec 3, 2018

Conversation

arkogupta
Copy link
Contributor

@arkogupta arkogupta commented Nov 30, 2018 •
edited
Loading

Pull request checklist

Description of changes

This change adds support for dragging and dropping columns to reorder them, in RTL languages.

Microsoft Reviewers: Open in CodeFlow

@natalieethell
Copy link
Contributor

Hey @arkogupta I just tested out your changes locally and I'm not seeing the right behavior for the DetailsList Drag & Drop example in RTL.

detailslistdragdroprtlnotworking

It looks like the columns are still either jumping to the far left or the far right.

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Nov 30, 2018 •
edited
Loading

Hmmm... unsure why @natalieethell ran into the above 🤔. In my local testing this seems to work well. Here is a test pass in Edge:

dl_drag_and_drop_rtl

Both the column positions and drop anchor markers seem correct. Will sync with @natalieethell offline to see what the culprit is.

KevinTCoughlin
KevinTCoughlin approved these changes Nov 30, 2018
View reviewed changes
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.

Change LGTM in my testing (both LTR/RTL) :shipit:

Will sync with @natalieethell to confirm it also works for her, please wait for that to merge.

@natalieethell
Copy link
Contributor

Update: got it working! Just synced up with @KevinTCoughlin offline and it works for me when I click the "Render in RTL" option on the gear icon in the top right corner of the demo page. Looks good.

(Before, I was just changing the <html dir="ltr"> tag at the top of the page to <html dir="rtl">. That didn't seem to re-load everything correctly.)

I'll approve this, but could we resolve the comment above before merging? Thanks!

@KevinTCoughlin KevinTCoughlin merged commit 1121afe into microsoft:master Dec 3, 2018
@msft-github-bot
Copy link
Contributor

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

Handy Links:

@msft-github-bot msft-github-bot mentioned this pull request Dec 4, 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

@natalieethell natalieethell natalieethell approved these changes

@KevinTCoughlin KevinTCoughlin KevinTCoughlin approved these changes

Assignees

@KevinTCoughlin KevinTCoughlin

Labels
Component: DetailsList
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Column drag-drop not working in RTL
4 participants
@arkogupta @natalieethell @KevinTCoughlin @msft-github-bot