Fix/tombo/responsive list issues #173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mentaman
commented
Jul 25, 2021
| menuButtonProps | ||
| }, | ||
| ref | ||
| ) => { | ||
| const componentRef = useRef(null); | ||
| const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); | ||
| const index = useElementsOverflowingIndex({ ref: componentRef, children, paddingSize, resizeDebounceTime }); | ||
| const index = useElementsOverflowingIndex({ |
There was a problem hiding this comment.
From what I saw we're the only one using ResponsiveList and useElementsOverflowingIndex
so there's no risk of breaking changes right?
There was a problem hiding this comment.
it is open source, so it not known, but this is still a work in progress, i don't mind changing it
mentaman
commented
Jul 25, 2021
| } | ||
|
|
||
|
|
||
| export const ChangeChildrenCheck = () => { |
There was a problem hiding this comment.
Without the fix, this story is broken.
If you prefer another solution and want to reproduce the bug,
you can copy the stories file here and open this story.
orrgottlieb
approved these changes
Jul 25, 2021
| menuButtonProps | ||
| }, | ||
| ref | ||
| ) => { | ||
| const componentRef = useRef(null); | ||
| const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); | ||
| const index = useElementsOverflowingIndex({ ref: componentRef, children, paddingSize, resizeDebounceTime }); | ||
| const index = useElementsOverflowingIndex({ |
There was a problem hiding this comment.
it is open source, so it not known, but this is still a work in progress, i don't mind changing it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were a few issues with the ResponsiveList we found, that caused it to break sometime and draw too much items.
It didn't draw items that it didn't have place for, so after it calculated their width once,
it wouldn't be able to calculate it again.
since the children here would include only part of the children
If there were new items added after drawing it, it wouldn't be able to calculate the widthes for the same reason and it'll break
To solve it, I'm drawing it twice now, once for calculation only invisibly with position absolute with the parent width,
and once the actual drawing.