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

fix(Dropdown): colliding toggle button and scroll #2006

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jurokapsiar
Copy link
Contributor

Before:
image

After:
image

@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2006   +/-   ##
=======================================
  Coverage   77.01%   77.01%           
=======================================
  Files         158      158           
  Lines        5456     5456           
  Branches     1578     1594   +16     
=======================================
  Hits         4202     4202           
  Misses       1241     1241           
  Partials       13       13

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 f988f34...cf35594. Read the comment docs.

@@ -106,7 +106,7 @@ const dropdownStyles: ComponentSlotStylesPrepared<DropdownPropsAndState, Dropdow
overflowY: 'auto',
maxHeight: v.selectedItemsMaxHeight,
width: '100%',
...(p.toggleIndicator && { paddingRight: v.toggleIndicatorSize }),
...(p.toggleIndicator && { marginRight: v.toggleIndicatorSize }),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this actually breaks IE in a strange way. Follwoing code added to the toggle indicator slot works better:

 root: {
   [`& .${Dropdown.slotClassNames.toggleIndicator}`]: {
     marginRight: "12px"
   }
  }

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jurokapsiar what is the strange behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the new styling moves the scrollbar for all browsers and not just IE11. I don't think that is what we will want, but i can't load in ie11 right nowso i don't understand the issue...

@ecraig12345 ecraig12345 mentioned this pull request Dec 6, 2019
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.

2 participants