-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handling Dropdown content resize #70
Conversation
@@ -111,6 +94,38 @@ class Switch extends React.PureComponent { | |||
} | |||
} | |||
|
|||
export default React.forwardRef((props, ref) => ( | |||
<Switch innerRef={ref} {...props} /> | |||
const basePropTypes = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dprzybylek small refactor to make default props and prop types of Switch visible in the docs
We should avoid defining those as static properties on a component class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok 👍
SwitchComponent.propTypes = { | ||
...basePropTypes, | ||
innerRef: PropTypes.instanceOf( | ||
typeof Element === 'undefined' ? () => {} : Element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dprzybylek take a look at #43
We need to figure out a better way to avoid SSR issues.
@@ -141,7 +141,8 @@ | |||
"react-day-picker": "^7.2.4", | |||
"react-material-icon-svg": "1.7.0", | |||
"react-popper": "^1.3.3", | |||
"react-transition-group": "^2.4.0" | |||
"react-transition-group": "^2.4.0", | |||
"resize-observer-polyfill": "^1.5.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/que-etc/resize-observer-polyfill
It has really good browsers support.
7dc4404
to
9c4fb47
Compare
f406c3c
to
ab6591c
Compare
super(props); | ||
|
||
this.state = { | ||
focusedElement: (props.items[0] && props.items[0].itemId) || null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
autofocus first item on component create
@@ -111,6 +94,38 @@ class Switch extends React.PureComponent { | |||
} | |||
} | |||
|
|||
export default React.forwardRef((props, ref) => ( | |||
<Switch innerRef={ref} {...props} /> | |||
const basePropTypes = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok 👍
@@ -34,7 +34,7 @@ exports[`Archives | Components | FiltersMenu renders correctly 1`] = ` | |||
/> | |||
} | |||
isDisabled={false} | |||
isFocused={false} | |||
isFocused={true} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so since now dropdown is default focused?
btw Archives | Components | FiltersMenu
this test should have another name ;)
and we should avoid renders correctly
tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's related to autofocus feature, this is the first available dropdown list item (not disabled).
I will change test name :P shame on me
className: PropTypes.string, | ||
defaultFocusedItemId: PropTypes.oneOfType([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where this prop is used? I can't find it in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover, good catch :)
We noticed the issue in Dropdown component. When the number of dropdown list items changes or items are removed on click (screen) Dropdown is not updating its position. Dynamic content of dropdown could also trigger this bug.
We decided to listen on Dropdown content wrapper resize to properly handle those cases.
References:
Also I added props for custom keycodes setup and autofocus first item on creating component and list items count change (filtering)