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

FocusTrapZone - restore last focused descendant element #5103

Merged
merged 18 commits into from
Jun 6, 2018
Merged

FocusTrapZone - restore last focused descendant element #5103

merged 18 commits into from
Jun 6, 2018

Conversation

bworline
Copy link
Contributor

@bworline bworline commented Jun 5, 2018

Added FocusTrapZone capability: When FTZ.focus() is called, it will pass focus to a descendant element. The new prop 'focusPreviouslyFocusedInnerElement' controls the descendant-choosing behavior, allowing you to focus either the first descendant or the previously focused descendant.

Unit tests added.

Microsoft Reviewers: Open in CodeFlow

@bworline bworline changed the title Ftz rememberlastfocused FocusTrapZone - restore last focused descendant element Jun 5, 2018
@bworline
Copy link
Contributor Author

bworline commented Jun 5, 2018

@cliffkoh Can you review?

Copy link
Contributor

@cliffkoh cliffkoh left a comment

Choose a reason for hiding this comment

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

Looks awesome to me. It would be great to get one more approval from a "scenario owner".

}

it('goes to previously focused element when focusing the FTZ', async () => {
expect.assertions(4);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@bworline
Copy link
Contributor Author

bworline commented Jun 5, 2018

@jspurlin , I removed all auto-focusing when the control gets focus (w.r.t. the previous version of this PR). Now it only does its thing when .focus() is called. Can you take a look?

@bworline
Copy link
Contributor Author

bworline commented Jun 6, 2018

@jspurlin is added to the review. #Closed

Copy link
Collaborator

@jspurlin jspurlin left a comment

Choose a reason for hiding this comment

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

Can you add the validation you performed for this PR?

@bworline
Copy link
Contributor Author

bworline commented Jun 6, 2018

Validation: Unit tests cover added behavior around focusing when focus() is called. Checked word-online demo app for regressions in ribbon.

@jspurlin jspurlin merged commit e307170 into microsoft:master Jun 6, 2018
@bworline bworline deleted the ftz-rememberlastfocused branch June 6, 2018 02:57
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Jun 6, 2018
* master:
  Part 2 of demo page refactor (microsoft#5089)
  Update jest.js
  fixing official example page and datepicker/calendar components using… (microsoft#5108)
  Don't run prettier and tslint in parallel as it might cause conflicts (microsoft#5100)
  FocusTrapZone - restore last focused descendant element (microsoft#5103)
  Coachmark/TeachingBubble: Fix SCSS selectors for buttons and Close Icon (microsoft#4835)
  HoverCard: IE11 fix (microsoft#5105)
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Jun 6, 2018
* master:
  Charting (microsoft#4954)
  Deprecation lint rule! (microsoft#5109)
  Implement selection for selected items list (microsoft#5036)
  Ignore common/changes and don't prettify json files (microsoft#5112)
  Part 2 of demo page refactor (microsoft#5089)
  Update jest.js
  fixing official example page and datepicker/calendar components using… (microsoft#5108)
  Don't run prettier and tslint in parallel as it might cause conflicts (microsoft#5100)
  FocusTrapZone - restore last focused descendant element (microsoft#5103)
  Coachmark/TeachingBubble: Fix SCSS selectors for buttons and Close Icon (microsoft#4835)
  HoverCard: IE11 fix (microsoft#5105)
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Jun 6, 2018
* master: (31 commits)
  Charting (microsoft#4954)
  Deprecation lint rule! (microsoft#5109)
  Implement selection for selected items list (microsoft#5036)
  Ignore common/changes and don't prettify json files (microsoft#5112)
  Part 2 of demo page refactor (microsoft#5089)
  Update jest.js
  fixing official example page and datepicker/calendar components using… (microsoft#5108)
  Don't run prettier and tslint in parallel as it might cause conflicts (microsoft#5100)
  FocusTrapZone - restore last focused descendant element (microsoft#5103)
  Coachmark/TeachingBubble: Fix SCSS selectors for buttons and Close Icon (microsoft#4835)
  HoverCard: IE11 fix (microsoft#5105)
  FocusTrapZone bug allows breaking out of the trap (microsoft#4898)
  Applying package updates.
  Update ISSUE_TEMPLATE.md
  Experiment/Nav component: hide nav group header if all the links under it are hidden (microsoft#5095)
  Add optional prop to not dismiss Callout on focus loss (microsoft#5092)
  Experiments: moves ShimmerTile from Shimmer to Tile. (microsoft#5090)
  Run jest in parallel on Windows (microsoft#5096)
  Applying package updates.
  Major bump jest-serializer-merge-styles
  ...
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants