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

fix(Chat): fix handling of onMouseEnter#1903

Merged
layershifter merged 3 commits intomasterfrom
fix/chat-message-override
Sep 9, 2019
Merged

fix(Chat): fix handling of onMouseEnter#1903
layershifter merged 3 commits intomasterfrom
fix/chat-message-override

Conversation

@layershifter
Copy link
Member

@layershifter layershifter commented Sep 9, 2019 •
edited
Loading

💥 Issue

Previously onMouseEnter was not handled properly, if you will pass custom onMouseEnter the position update will be never called.

@layershifter layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Sep 9, 2019
@codecov
Copy link

codecov bot commented Sep 9, 2019 •
edited
Loading

Codecov Report

Merging #1903 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1903      +/-   ##
==========================================
+ Coverage   69.87%   69.89%   +0.02%     
==========================================
  Files         890      890              
  Lines        7783     7785       +2     
  Branches     2275     2275              
==========================================
+ Hits         5438     5441       +3     
+ Misses       2335     2334       -1     
  Partials       10       10
Impacted Files Coverage Δ
packages/react/src/components/Chat/ChatMessage.tsx 75.51% <100%> (+3.16%) ⬆️

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 ed2d996...063a2c2. Read the comment docs.

@lucivpav
Copy link
Contributor

lucivpav commented Sep 9, 2019

But how could you previously pass onMouseEnter, when ChatMessage didn't have such prop? Shouldn't this be a feature PR, that adds such a prop?

lucivpav
lucivpav reviewed Sep 9, 2019
View reviewed changes
packages/react/test/specs/components/Chat/ChatMessage-test.tsx
focusZoneDefinition: (chatMessageBehavior as AccessibilityDefinition).focusZone,
})
})

Copy link
Contributor

@lucivpav lucivpav Sep 9, 2019

Choose a reason for hiding this comment

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

How about another test, that will check that a custom onMouseEnter function gets called?

Copy link
Member Author

@layershifter layershifter Sep 9, 2019

Choose a reason for hiding this comment

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

It's already tested by isConformant test, see handles events transparently.

@layershifter @lucivpav
Update packages/react/src/components/Chat/ChatMessage.tsx
063a2c2 
Co-Authored-By: Pavel Lučivňák <pavel.lucivnak@gmail.com>
@layershifter
Copy link
Member Author

layershifter commented Sep 9, 2019 •
edited
Loading

But how could you previously pass onMouseEnter, when ChatMessage didn't have such prop?

It was overridden as an unhandled prop:
image

@layershifter layershifter merged commit 2eeb285 into master Sep 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/chat-message-override branch September 9, 2019 15:20
layershifter added a commit that referenced this pull request Sep 13, 2019
@layershifter
fix(Chat): fix handling of onMouseEnter (#1903)
ee7972b 
* fix(Chat): fix handling of `onMouseEnter`

* add changelog entry

* Update packages/react/src/components/Chat/ChatMessage.tsx

Co-Authored-By: Pavel Lučivňák <pavel.lucivnak@gmail.com>

(cherry picked from commit 2eeb285)
@layershifter layershifter mentioned this pull request Sep 13, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@kuzhelov kuzhelov Awaiting requested review from kuzhelov

@levithomason levithomason Awaiting requested review from levithomason levithomason is a code owner

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny miroslavstastny is a code owner

@mnajdova mnajdova Awaiting requested review from mnajdova

1 more reviewer

@lucivpav lucivpav lucivpav left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🧰 fix Introduces fix for broken behavior. 🚀 ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@layershifter @lucivpav