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

Changes to allow focus and keyboard interaction in close and back icons #4136

Merged
merged 9 commits into from
Nov 18, 2019
Merged

Changes to allow focus and keyboard interaction in close and back icons #4136

merged 9 commits into from
Nov 18, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Nov 5, 2019

Summary

Changes to allow focus and keyboard interaction in close and back icons for the full screen modal.

Ticket Link

MM-17536

@jespino jespino added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 5, 2019
@jespino jespino added this to the v5.18.0 milestone Nov 5, 2019
deanwhillier
deanwhillier suggested changes Nov 11, 2019
View reviewed changes
Copy link
Contributor

@deanwhillier deanwhillier left a comment

Choose a reason for hiding this comment

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

Pulling the back button up to this level is perfect! 👍

I have a few semantic/accessibility thoughts for your consideration.

components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
devinbinnie
devinbinnie suggested changes Nov 11, 2019
View reviewed changes
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Looks good, just one nitpick from me.

components/widgets/modals/full_screen_modal.tsx Outdated Show resolved Hide resolved
@srkgupta srkgupta added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Nov 12, 2019
srkgupta
srkgupta approved these changes Nov 12, 2019
View reviewed changes
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.

Tested on the PR instance and the changes looks good.

@srkgupta srkgupta added 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 Nov 12, 2019
@mattermod
Copy link
Contributor

Test server destroyed

@amyblais amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Nov 18, 2019
@jespino jespino removed the 2: Dev Review Requires review by a core commiter label Nov 18, 2019
@deanwhillier deanwhillier merged commit 2820cf3 into mattermost:master Nov 18, 2019
@mattermod
Copy link
Contributor

@jespino
Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-#4136-upstream-release-5.18-1574088524
Branch 'automated-cherry-pick-of-#4136-upstream-release-5.18-1574088524' set up to track remote branch 'release-5.18' from 'upstream'.
+++ Downloading patch to /tmp/4136.patch (in case you need to do this again)

+++ About to attempt cherry pick of PR. To reattempt:
  $ git am -3 /tmp/4136.patch

Applying: Changes to allow focus and keyboard interaction in close and back icons
Using index info to reconstruct a base tree...
M	components/invitation_modal/__snapshots__/invitation_modal.test.jsx.snap
M	components/invitation_modal/__snapshots__/invitation_modal_confirm_step.test.jsx.snap
M	components/invitation_modal/__snapshots__/invitation_modal_guests_step.test.jsx.snap
M	components/invitation_modal/__snapshots__/invitation_modal_members_step.test.jsx.snap
M	components/invitation_modal/invitation_modal.jsx
M	components/invitation_modal/invitation_modal_confirm_step.jsx
M	components/invitation_modal/invitation_modal_members_step.jsx
M	components/invitation_modal/invitation_modal_members_step.test.jsx
Falling back to patching base and 3-way merge...
Auto-merging components/invitation_modal/invitation_modal_members_step.test.jsx
Auto-merging components/invitation_modal/invitation_modal_members_step.jsx
Auto-merging components/invitation_modal/invitation_modal_confirm_step.jsx
CONFLICT (content): Merge conflict in components/invitation_modal/invitation_modal_confirm_step.jsx
Auto-merging components/invitation_modal/invitation_modal.jsx
Auto-merging components/invitation_modal/__snapshots__/invitation_modal_members_step.test.jsx.snap
Auto-merging components/invitation_modal/__snapshots__/invitation_modal_guests_step.test.jsx.snap
Auto-merging components/invitation_modal/__snapshots__/invitation_modal_confirm_step.test.jsx.snap
Auto-merging components/invitation_modal/__snapshots__/invitation_modal.test.jsx.snap
CONFLICT (content): Merge conflict in components/invitation_modal/__snapshots__/invitation_modal.test.jsx.snap
Patch failed at 0001 Changes to allow focus and keyboard interaction in close and back icons
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

+++ Conflicts detected:

UU components/invitation_modal/__snapshots__/invitation_modal.test.jsx.snap
UU components/invitation_modal/invitation_modal_confirm_step.jsx

+++ Aborting in-progress git am.

+++ Returning you to the master branch and cleaning up.

@jespino jespino added the 4: Reviews Complete All reviewers have approved the pull request label Nov 18, 2019
jespino added a commit that referenced this pull request Nov 18, 2019
@jespino
Changes to allow focus and keyboard interaction in close and back ico…
a6b66b7 
…ns (#4136)

* Changes to allow focus and keyboard interaction in close and back icons

* Fixing lint errors

* Using buttons to have fully semantic accesibility for back and close

* Add aria labels to the close and back buttons

* Fixing tests

* Fixing other snapshots

* Fixing snapshots
@jespino jespino added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Nov 18, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Nov 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@srkgupta srkgupta srkgupta approved these changes

@deanwhillier deanwhillier deanwhillier approved these changes

@devinbinnie devinbinnie devinbinnie approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation QA Review Done
Projects
None yet
Milestone
v5.18.0
6 participants
@jespino @mattermod @srkgupta @deanwhillier @devinbinnie @amyblais