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

fix(attachment): action click #1444

Merged
merged 3 commits into from
Jun 4, 2019
Merged

fix(attachment): action click #1444

merged 3 commits into from
Jun 4, 2019

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Jun 3, 2019

fix(attachment): action click

Description

This PR fixes #1334

Steps to reproduce issue:

  1. Click on the action icon in the attachment and notice that the action's onClick event is fired.
  2. Focus the Attachment, then click on the action icon and notice that the attachment's onClick event gets fired.

Fix

Add pointerEvents: 'none' to getBorderFocusStyles.ts style helper in order ignore pointer events for the pseudo elements.

BEFORE

The Attachment onClick event is fired when clicking the action icon when the attachment is focused.

attachment-onClick-bug

AFTER

Screen Recording 2019-06-03 at 14 43 59

@codecov
Copy link

codecov bot commented Jun 3, 2019

Codecov Report

Merging #1444 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1444   +/-   ##
======================================
  Coverage    73.6%   73.6%           
======================================
  Files         787     787           
  Lines        5907    5907           
  Branches     1744    1744           
======================================
  Hits         4348    4348           
  Misses       1553    1553           
  Partials        6       6
Impacted Files Coverage Δ
...ges/react/src/themes/teams/getBorderFocusStyles.ts 18.75% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 882f293...90aaa25. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 3, 2019

Codecov Report

Merging #1444 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1444   +/-   ##
=======================================
  Coverage   73.14%   73.14%           
=======================================
  Files         805      805           
  Lines        6073     6073           
  Branches     1775     1794   +19     
=======================================
  Hits         4442     4442           
  Misses       1625     1625           
  Partials        6        6
Impacted Files Coverage Δ
...ges/react/src/themes/teams/getBorderFocusStyles.ts 18.75% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5303cdd...ab6f9ed. Read the comment docs.

@bmdalex bmdalex force-pushed the fix/attachment-action-click branch from 03386c7 to c95241d Compare June 3, 2019 16:16
@@ -22,6 +22,7 @@ const getPseudoElementStyles = (args: BorderPseudoElementStyles): ICSSInJSStyle
content: '""',
position: 'absolute',
borderStyle: 'solid',
pointerEvents: 'none',
Copy link
Contributor

Choose a reason for hiding this comment

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

@kolaps33 please test this with screen readers

@bmdalex bmdalex force-pushed the fix/attachment-action-click branch from c95241d to ab6f9ed Compare June 4, 2019 09:22
@bmdalex bmdalex merged commit b5ad236 into master Jun 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/attachment-action-click branch June 4, 2019 09:57
Copy link
Collaborator

@kolaps33 kolaps33 left a comment

Choose a reason for hiding this comment

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

tested with VoiceOver looks ok :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attachment's action icon button onclick isn't working correctly when attachment is focused
4 participants