-
Notifications
You must be signed in to change notification settings - Fork 2
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
✨(react) Datepicker position #133
Conversation
🦋 Changeset detectedLatest commit: 419d61e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
22f0a73
to
8396d68
Compare
38f9330
to
35240d7
Compare
f773486
to
669d8ab
Compare
669d8ab
to
4d65cfd
Compare
Some scripts had errors when running on windows. We now use some cross platform cli tools to be able to run the scripts on every platform.
If we work from an app perspective, it is nice to have a watch mode on our packages to see the changes in real time. Better to use the watch mode of vite (rollup) compare to nodemon because it is faster, we re-transpile only the files that have changed. Possility to use the wath mode by polling as well, on a remote machine the HMR does not work well, the polling mode helps to solve this issue.
Eslint extension highlighted some issues with the way we were importing the tsconfig.json file in the .eslintrc.json file. By changing the extension to .cjs, we are now able to set tsconfigRootDir to the root of the package thanks to `dirname`. It helps eslint to find the correct tsconfig.json file.
931b8c6
to
757cfb6
Compare
Prettier had some missing extensions in the config, this commit added them, we then prettified the whole project. We ignore the `cunningham-tokens` files.
08b7d9d
to
1a6db4e
Compare
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 few nit-picking remarks 🌟 and a conversation about the refined UX 👀
Are the following behaviors expected? Let's discuss it; there might be an issue within the code. The popover position should update upon scrolling and close when necessary. WDYT?
Enregistrement.de.l.ecran.2023-08-17.a.22.08.46.mov
Enregistrement.de.l.ecran.2023-08-17.a.22.08.03.mov
In general, @jbpenrath, should we establish a standardized branching model for the project? I had it on my to-do list to initiate a conversation regarding this in the Tech channel. I'm more than willing to take that step.
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.
LGTM 🤗
Position the datepicker on top of the input when the input is at the bottom of the screen and whenthere is enough space to display the datepicker on the top.
ce80a59
to
419d61e
Compare
Purpose
We have a problem on marsha and LTI with the datepicker not accessible (see conversation).
The goal is to change the position of the datepicker depend of the place available in the screen.
Proposal
If not enough place on the bottom and enough place on the top, we place it to the top, otherwise to the bottom.
Vite.+.React.+.TS.webm
Review
Extra check here, I don't think that you need nodemon to watch the project when you do
yarn dev
on react packages, the vite HMR seems to be activated by default when you startstorybook
, but maybe I misinterpreted something.Not sure if it is the correct way to do:
https://github.com/openfun/cunningham/blob/8396d6892f5b4a5186ad05e21c71db4c16e43008/.changeset/wet-keys-occur.md