MEN-8001 #172
MEN-8001 #172
Conversation
Signed-off-by: Manuel Zedel <[email protected]>
Signed-off-by: Manuel Zedel <[email protected]>
…sorting function Signed-off-by: Manuel Zedel <[email protected]>
| if (typeof a[field] === 'string') { | ||
| const result = a[field].localeCompare(b[field], { sensitivity: 'case' }); | ||
| const result = a[field].localeCompare(b[field], navigator.language, { sensitivity: 'case' }); |
There was a problem hiding this comment.
I think it's not the best idea to sort it based on browser settings. Maybe en-US?
There was a problem hiding this comment.
it's what the localeCompare did until now if I read https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare#locales correctly - just that the absence of the parameter made the sensitivity option not apply... maybe it would be clearer with undefined?
There was a problem hiding this comment.
Well, undefined is effectively equivalent to navigator.language in this case. en-US will sort things consistently across browsers, but maybe wrong in different alphabet. I am not sure what is more important here
There was a problem hiding this comment.
en-US won't sort it consistent with the user expectation either if the expectation is based on how it works now 🤷 - IMO the important part is getting the case insensitivity enabled, as that was intentional in here
packages/utils/src/helpers.ts
Outdated
| import dayjs from 'dayjs'; | ||
| import utc from 'dayjs/plugin/utc.js'; | ||
| import pluralize from 'pluralize'; | ||
| import Cookies from 'universal-cookie'; | ||
|
|
||
| dayjs.extend(utc); | ||
|
|
||
| const isEncoded = uri => { | ||
| const isEncoded = (uri: string) => { | ||
| uri = uri || ''; |
There was a problem hiding this comment.
Not related to your changes, but this could be achieved through default params. I think it's better to avoid reassigning the arguments
There was a problem hiding this comment.
we can't default to this everywhere though, as we might get a falsy value that's not undefined passed (thinking primarily about the inputs where we fallback to empty strings)
| hasMonitor: boolean; | ||
| ipAddress?: string; | ||
| isDemoMode: boolean; | ||
| isHosted: boolean; |
There was a problem hiding this comment.
do you think it would make sense to add member ordering rules to eslint?
There was a problem hiding this comment.
yeah, I was tempted to do that, but we rely on the order of some objects in the UI code, where we then iterate over the object entries... but since the iteration order afaik isn't guaranteed, we could add the rule and either ignore the places with expected order or make the iteration rely on a stable index 👌 => will do this in a follow up PR though
There was a problem hiding this comment.
I think it's possible to apply rules to typescript types and interfaces definitions exclusively, so it won't affect js objects, but both things are nice to have 👍
There was a problem hiding this comment.
#174 turns out the ts rule only is for ts things 😆 - so the regular object sorting order can be taken on a later date 👍
Signed-off-by: Manuel Zedel <[email protected]>
No description provided.