-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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
[CHANGELOG] Prepare v5.0.0-alpha.23 #24406
Conversation
mnajdova
commented
Jan 14, 2021
- I have followed (at least) the PR section of the contributing guide.
- [TimePicker] Add pointer cursor for clock in desktop (#24276) @praveenkumar-kalidass | ||
- [lab] Drop usage of createStyles (#24158) @eps1lon | ||
- [lab] Fix import paths in generated declaration files (#24380) @eps1lon | ||
- [lab] Prevent possible null pointer in useValidation (#24318) @eps1lon |
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.
This has a narrower impact, and is not about the lab package per say
- [lab] Prevent possible null pointer in useValidation (#24318) @eps1lon | |
- [DatePicker] Prevent possible null pointer in useValidation (#24318) @eps1lon |
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.
These are still not only targetting the DatePicker.
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.
"DatePicker" has been used, so far, as a placeholder for all the date picker components (when something more specific doesn't exist).
We could make them inside of [core] in this case, if you consider it's internal.
[lab] is about change that are specific to the lab package, like fixing the TypeScript issue because we have a lab package and we don't have the components in the core package
[core] is about all the devops work and code infrastructure that supports the public modules.
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 not devops. It's about components in the lab.
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 it's about the date pickers, at large? This mention in the changelog would create a precedent. I think that we should optimize for what the developers reading the changelog would want to know. From my perspective, it seems more relevant to know that it's linked to a better date picker (and all its variants). I assume that the npm packages core/unstyled/styled/etc. are only means to bundle and distribute the different modules.
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 it's about the date pickers, at large?
Date and time pickers. Not just date pickers.
I think that we should optimize for what the developers reading the changelog would want to know.
We agree on that
it seems more relevant to know that it's linked to a better date picker (and all its variants).
A time picker is not a date picker.
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.
A time picker is not a date picker.
I guess our disagreement comes from this, I was considering it to be close enough.
- Semantically and conceptually close
- JavaScript
new Date()
. - Time picker is not was most developers are looking for. I think that we will more accurate state once v5 is released as stable and we can look at GA page views, but early data I could get my hand on would suggest the
TimePicker
is about 25% of the need compared to theDatePicker
.
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.
Should we then leave this as lab
at this moment and merge? I'll start with the release right away
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.
Maybe we could create a new label that includes DatePicker
, TimePicker
, DateRangePicker
, DateTimePicker
. Historically, we have used Pickers
but it's a bit confusing without more context.
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>