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

fix: (ListItem) - Fix endMedia to not be removed from DOM on mouseleave for ListItem #278

Merged
merged 16 commits into from
Oct 31, 2018

Conversation

musingh1
Copy link
Contributor

Earlier, we were listening for mouseEnter and mouseLeave, maintaining the isHovering state and only showing the endMedia when the item is hovered. This breaks the scenarios where endMedia is a Popup. Once the Popup is triggered we want the content to stay even after the user leaves the list item (e.g. points to the popup content).
So now, if provided we will always render all the media's and in Teams theme control the display of endMedia, headerMedia etc through styles.

@codecov
Copy link

codecov bot commented Sep 25, 2018

Codecov Report

Merging #278 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #278      +/-   ##
==========================================
+ Coverage   91.72%   91.77%   +0.05%     
==========================================
  Files          41       41              
  Lines        1341     1325      -16     
  Branches      172      194      +22     
==========================================
- Hits         1230     1216      -14     
+ Misses        107      105       -2     
  Partials        4        4
Impacted Files Coverage Δ
src/components/List/ListItem.tsx 100% <ø> (+6.38%) ⬆️
src/components/Portal/Portal.tsx 96.29% <0%> (-1.86%) ⬇️
src/components/Status/Status.tsx 100% <0%> (ø) ⬆️

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 183c250...7a98de0. Read the comment docs.

@musingh1 musingh1 changed the title fix(ListItem) - Fixing endMedia to not be removed from DOM on mouseleave for ListItem fix(ListItem) - Fixed endMedia to not be removed from DOM on mouseleave for ListItem Sep 25, 2018
@musingh1 musingh1 changed the title fix(ListItem) - Fixed endMedia to not be removed from DOM on mouseleave for ListItem fix(ListItem) - Fix endMedia to not be removed from DOM on mouseleave for ListItem Sep 25, 2018
CHANGELOG.md Outdated Show resolved Hide resolved
@jurokapsiar
Copy link
Contributor

@musingh1 the same would need to be done for focus state if the user is using keyboard. Would you mind adding following commit to the PR: feature/mukul/fix-listitem-styles...feat/listitem-focus-styles

or should we create a followup PR after this one is merged?

@musingh1 musingh1 changed the title fix(ListItem) - Fix endMedia to not be removed from DOM on mouseleave for ListItem fix: (ListItem) - Fix endMedia to not be removed from DOM on mouseleave for ListItem Oct 2, 2018
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kuzhelov kuzhelov left a comment

Choose a reason for hiding this comment

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

lets address comments before merging that.

Also, there are couple problems appearing when List Item example is rendered in Docs:

  • Received true for a non-boolean attribute selection. ..
  • child in an array or iterator should have a unique "key" prop

Lets address them as well before merging (I may assist in that :).

@kuzhelov kuzhelov added the needs author feedback Author's opinion is asked label Oct 4, 2018
@musingh1 musingh1 removed the needs author feedback Author's opinion is asked label Oct 24, 2018
@musingh1 musingh1 merged commit 3499658 into master Oct 31, 2018
@layershifter layershifter deleted the feature/mukul/fix-listitem-styles branch November 9, 2018 09:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants