Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Add full screen modal focus trap #4140

Merged
merged 29 commits into from
Dec 17, 2019
Merged

Add full screen modal focus trap #4140

merged 29 commits into from
Dec 17, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Nov 5, 2019

Summary

Add full screen modal focus trap

Ticket Link

MM-17534

@jespino jespino added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Nov 5, 2019
@hanzei hanzei added the Work in Progress Not yet ready for review label Nov 8, 2019
@jespino jespino requested review from a team and lindy65 November 18, 2019 21:04
@ghost ghost requested review from marianunez and mgdelacroix and removed request for a team November 18, 2019 21:04
@jespino jespino added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester and removed Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) Work in Progress Not yet ready for review labels Nov 18, 2019
@jespino jespino added this to the v5.18.0 milestone Nov 18, 2019
@jespino jespino added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Nov 18, 2019
@lindy65 lindy65 requested review from srkgupta and removed request for lindy65 November 19, 2019 08:11
@jespino jespino requested a review from srkgupta December 3, 2019 15:37
@srkgupta srkgupta removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Dec 4, 2019
@mattermod
Copy link
Contributor

Test server destroyed

@srkgupta
Copy link
Contributor

srkgupta commented Dec 4, 2019

Removing the Cloud Test Server. Will need to confirm the scope of this PR with PM. Will pick it up once its confirmed.

@jespino
Copy link
Member Author

jespino commented Dec 15, 2019

/update-branch

@srkgupta srkgupta added Setup Cloud Test Server Setup a test server using Mattermost Cloud and removed Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Dec 16, 2019
@mattermod
Copy link
Contributor

Test server destroyed

@srkgupta srkgupta added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Dec 16, 2019
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Hi @jespino
Following issues are still seen. Can you please check and fix them:
2. When the Invite People Modal is opened, it should be announced like any other dialogs (Manage Channels or Manage Team Members). Currently the JAWS reader reads "Left Bracket Object Object Right Bracket modal dialog when this is opened.
3. F6 or Ctrl+F6 still does not works on the Invite People modal. Currently it reads out the elements in the background of the modal. F6 or Ctrl+F6 should work like TAB in case of this modal.

@jespino
Copy link
Member Author

jespino commented Dec 16, 2019

@srkgupta I have fixed the point 2, the point 3 I have no idea how it works, probably we should defer it to @devinbinnie or @deanwhillier, I took a look of the previous F6 related changes, and I didn't found how to apply that to my PR, I think is not trivial and have relation with sections.

@mattermod
Copy link
Contributor

Mattermost test server updated with git commit 8a9822caa0ee0c90b8156dfb200fb7382730fde6.

Access here: https://mattermost-webapp-pr-4140.test.mattermost.cloud

Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Thanks @jespino. Added all the remaining issues in the following ticket.
https://mattermost.atlassian.net/browse/MM-17534

Approving the PR. Testcase added to the release spreadsheet.

@srkgupta srkgupta added 4: Reviews Complete All reviewers have approved the pull request QA Review Done and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Dec 17, 2019
@mattermod
Copy link
Contributor

Test server destroyed

@srkgupta
Copy link
Contributor

@jespino Some checks are unsuccessful. Can you please check, resolve and merge the PR.

@jespino jespino self-assigned this Dec 17, 2019
@jespino jespino merged commit 1131650 into mattermost:master Dec 17, 2019
@jespino jespino deleted the MM-17534 branch December 17, 2019 12:34
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation QA Review Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants