[Unstable_Gridv2] sorted responsize keys based on breakpoint value #34987
Conversation
|
| const keys = ( | ||
| Object.keys(responsize).length > breakpoints.keys.length | ||
| ? breakpoints.keys | ||
| : Object.keys(responsize); | ||
| : Object.keys(responsize) | ||
| ).sort((key1, key2) => { | ||
| // @ts-ignore already checked that responsize is an object | ||
| const breakpointValue1: T = breakpoints.values[key1]; | ||
| // @ts-ignore already checked that responsize is an object | ||
| const breakpointValue2: T = breakpoints.values[key2]; | ||
| if (typeof breakpointValue1 === 'number' && typeof breakpointValue2 === 'number') { | ||
| return breakpointValue1 - breakpointValue2; | ||
| } | ||
| return 0; | ||
| }); |
There was a problem hiding this comment.
Can you create a util function in this file with a test to make sure that it works as expected?
Something like this:
const keys = sortBreakpoints(breakpoints, responsive);There was a problem hiding this comment.
Yeah sure I'll do it
There was a problem hiding this comment.
I like the fix from https://github.com/mui/material-ui/pull/34985/files, it looks simple. Can you do that with a test?
There was a problem hiding this comment.
ok sure, i'll do that and add test
There was a problem hiding this comment.
I like the fix from https://github.com/mui/material-ui/pull/34985/files, it looks simple. Can you do that with a test?
i've added the test and rewrote logic based on https://github.com/mui/material-ui/pull/34985/files to sort keys
siriwatknp
added
bug 🐛
|
@Falkyouall tag you as a reviewer. |
sai6855
changed the title
|
hey thanks for approval, looking forward for more contribution |
|
hey @siriwatknp any idea when this PR can be merged or why this PR hasn't been merged yet? |
…ui#34987) Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
…ui#34987) Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
…ui#34987) Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
Fixes : #34928
Proof: https://codesandbox.io/s/vigorous-morning-3d8wq4