-
Notifications
You must be signed in to change notification settings - Fork 675
improved the date range selector and fixed #54 #60 #64
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
Conversation
Hey @Abaali I like this solution a lot! There are a couple of UI/UX things that I think would make it better, however I'm not sure if they are all possible. Improvement 1We should change the date formate to be YYYY-mm-dd HH:MM 2021-19-01 04:30 PM --> 2021-01-19 04:30 PM Improvement 2This one is a little trickier, but I think if we use these date pickers that select a range we have to have it affect both dates. Is it possible to make just one daterange selector affect both start date and end date? Right now its confusing because when you move the mouse around while selecting an end date, it seems like you are selecting a full range because a full range gets highlighted. Here's a gif where I'm only adjusting the start date, but when I change the start date from the Jan 15 to Jan 1, it looks as if I'm changing the full range to be: Do you think we could get ours to function more like this one? |
@LouisInFlow I added some small changes
the selector can be changed to look like the one you mentioned, but it will need a little work. So I suggest merging this PR to solve the current issues until we find a better way to implement it. if you or someone else can handle the CSS part, then I can change the package we are using now with this one. |
Hey @Abaali the thing stopping us from merging this version is that the UI/UX is actually more confusing than the version before. The reason why, is because in this version there are two colors, Grey and Blue... The problem is that these colors don't consistently mean something. In the example I linked the meaning of the colors is very straightforward:
In the version from this pr:
Here is what I propose in order to merge this PR:
So this:
One more example: And here the dates February 12 - February 26 should just be normal white UNTIL the user clicks: Then after the user clicks the range should look like this picture except the range February 1 through February 26 should be blue instead of grey: Does that make sense? |
I worked on a quick solution check it out |
Hey @Abaali yeah this is great! If you can resolve the conflicts then we can merge this |
conflicts resolved! :) |
Thanks for this @Abaali Pyroscope's whole flow is much smoother thanks to your last couple PRs 🙌🏾 |
You're welcome 😄 |
this PR fixes #54 and #60
any remarks about the code or the comments let me know
I tested the functionality before this pr but if something isn't working open an issue or mention it here so I can fix it