fix(Dropdown): Support for configuring dropdown offset #17
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
=======================================
Coverage 74.21% 74.21%
=======================================
Files 13 13
Lines 159 159
Branches 24 23 -1
=======================================
Hits 118 118
- Misses 30 31 +1
+ Partials 11 10 -1
Continue to review full report at Codecov.
|
src/Dropdown/Dropdown.js
Outdated
| @@ -71,7 +72,7 @@ export default class Dropdown extends React.Component { | |||
| } | |||
|
|
|||
| show = () => { | |||
| const { placement, boundariesElement } = this.props; | |||
| const { offset = '0, 10', placement, boundariesElement } = this.props; | |||
There was a problem hiding this comment.
add offset to propTypes and defaultProps?
There was a problem hiding this comment.
Thanks @choochootrain, great idea. Updated.
src/Dropdown/Dropdown.js
Outdated
| @@ -42,6 +42,7 @@ export default class Dropdown extends React.Component { | |||
| placement: 'bottom-start', | |||
| boundariesElement: 'window', | |||
| on: 'click', | |||
| border: true, | |||
|
Can we just configure the dropdown styles in cms |
|
Thanks @kylealwyn, that's a better approach -- we can specify the styles in theme instead. An offset will still need to be passed down in order to specify how Popper.js will handle/calculate placement. |
cehsu
changed the title
| @@ -82,7 +86,7 @@ export default class Dropdown extends React.Component { | |||
| placement, | |||
| modifiers: { | |||
| offset: { | |||
| offset: '0, 10', | |||
There was a problem hiding this comment.
Does this need to be offset: this.props.offset? Or pull out offset from props spread on L78
There was a problem hiding this comment.
Yes! That was the intention, thanks for catching the oversight, fixed.
There was a problem hiding this comment.
Was this tested locally? Browser would've thrown
There was a problem hiding this comment.
I hadn't tested it locally, is something like yalc the best tool for that? Or by running the docs command?
There was a problem hiding this comment.
Thanks for the reminder Kyle, screenshot added to description above. (I had been testing the changes locally by editing the molekule.es.js file in cms/node_modules, and had forgotten to copy over the assignment.)
There was a problem hiding this comment.
Looks like you found yarn docs :)
kylealwyn
changed the title
The redesigned EMR dashboard requires the following properties of the dropdown component to be configurable: border-radius, border, offset.
Related CMS PR: https://github.com/sappira-inc/cms/pull/225
Tested locally via
yarn docs