add shift into delete action#7274
Conversation
|
Should we be committing |
|
@FrancisMengx is it possible to add a unit test for this fix? |
cliffkoh
left a comment
cliffkoh
left a comment
There was a problem hiding this comment.
We should not check in the yarn.lock file.
@FrancisMengx from where are you getting the "Shift+Del" interaction? It is definitely not super intuitive to me ..
@joschect what do you think of the proposed change here?
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Add shift into delete suggestion.", | ||
| "type": "minor" |
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Add shift into delete suggestion.", |
There was a problem hiding this comment.
This should be something like Picker: ....
|
I'm pretty confused about this. What exactly is this fixing? Also what if I highlight something with shift, but don't have it pressed? |
|
@joschect https://developer.microsoft.com/en-us/fabric#/components/peoplepicker In here, you can repro:
Expect: Actual: The fix is to differentiate between the input editing delete and deleting suggestions. |
|
@cliffkoh shift delete is the keyboard short cut in windows deleting something permanently. |
…x/office-ui-fabric-react into add-shift-into-delete-action
Dismiss review as the changes requested has been made
cliffkoh
left a comment
cliffkoh
left a comment
There was a problem hiding this comment.
We still need @joschect to sign off on the new keyboard shortcut. I'm not sure to what extent this is considered a "breaking" change from Picker functionality but the API surface is otherwise similar.
@joschect is there a PM or designer for the Pickers who needs to get involved with this change? @FrancisMengx is making this change based off a request by a PM on OWA's end.
|
@cliffkoh I don't know of a designer who owns the people picker/pickers. This change makes sense as right now I would consider it broken, it's really bad that we'll delete suggestions without letting you know if you just press delete. |
…x/office-ui-fabric-react into add-shift-into-delete-action
|
🎉 Handy Links: |
* 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) ...
6 participants
Pull request checklist
$ npm run changeDescription of changes
Fix for current bug with delete button.
Repro of the bug:
Expect:
Only remove the character after the cursor.
Actual:
Remove both the character and highlighted option.
Focus areas to test
(optional)
Microsoft Reviewers: Open in CodeFlow