Skip to content
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

feat(list-page): add filtering options by timestamp #8965

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pnodet
Copy link
Contributor

@pnodet pnodet commented Dec 15, 2023

Currently in the list page we can't filter by dates.

To prevent issues with multiple nested popover (wrong behaviour of on click outside) I had to create a BlockDatePicker component (Calendar not wrapped in popover)

There might be a better way but it was the easiest non-breaking idea.

Screenshot 2023-12-15 at 18 04 45

Create a BlockDatePicker component (Calendar not wrapped in popover) to prevent issues with multiple nested popover (wrong behaviour of on click outside)
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8e816f7:

Sandbox Source
@keystone-6/sandbox Configuration

@pnodet pnodet closed this Dec 15, 2023
@pnodet pnodet reopened this Dec 15, 2023
@gautamsi
Copy link
Member

do you plan to add time field as well?

@pnodet
Copy link
Contributor Author

pnodet commented Dec 15, 2023

@gautamsi I'm not a maintainer of the repo, I don't even know if this PR will be accepted. I'm not sure what you mean by time fields? This works for dates / timestamps

@pnodet
Copy link
Contributor Author

pnodet commented Dec 15, 2023

@gautamsi Sorry missed that you were a member haha. Still not sure of what you mean, to be honest I built this because we needed it but I can add support for other fields I guess.

@pnodet
Copy link
Contributor Author

pnodet commented Dec 15, 2023

Btw I didn't see any prettier /formatter config, any suggestions?

@gautamsi
Copy link
Member

I am a member as part of restricted preview group long time ago, I have not much permission :(

I meant time part of this timestamp field, what you have is only date part from the screenshot.

@pnodet
Copy link
Contributor Author

pnodet commented Dec 15, 2023

@gautamsi Okay np. I think time would be a bit more challenging to implement, is it something you would use rather than filtering by date?

@gautamsi
Copy link
Member

I had to do this for a customer, I took different approach.

in v5 the timestamp field was using chrono-node for parsing humanized dates, I gave him that instead of dropdown.

@pnodet
Copy link
Contributor Author

pnodet commented Dec 15, 2023

I see… I think in our use case precise times are not really useful since we might be dealing with more long term objects
I'll think about it

@gautamsi
Copy link
Member

If you are not covering general use case I am not sure if this PR will be any useful,

This can be for reference for others to implement such custom view.

@pnodet
Copy link
Contributor Author

pnodet commented Dec 16, 2023

Yes sure took me a bit of time to understand how to implement this and maybe others will benefit from it.

What I meant was I need this feature so I built it in hope that it will be merged and that I don't have to maintain a fork etc.
I'd be willing to implement other filters has well but I re-used an existing component (the date picker) in order to make minimal changes. But you can surely implement chrono-node with the same logic

@gautamsi
Copy link
Member

you do not have to maintain a fork, you can use custom view which will replace the current filters, we have implemented several other type of filters with custom views for our clients

@pnodet
Copy link
Contributor Author

pnodet commented Dec 16, 2023

Oh so good, do you know where I can find some docs?

this look empty:
https://keystonejs.com/docs/guides/custom-field-views#title

@pnodet
Copy link
Contributor Author

pnodet commented Jan 2, 2024

@dcousens sorry for this pings, same question with this PR. Is this something you are inserted in / intend to merge?

@pnodet
Copy link
Contributor Author

pnodet commented Jan 25, 2024

@dcousens curious to have your reaction on this as well, when you have some time

@dcousens dcousens self-requested a review January 25, 2024 03:36
@dcousens
Copy link
Member

dcousens commented Jan 25, 2024

As this is a timestamp, we should show a complete timestamp.
A timestamp could default to 00:00:00 of the particular date selected.

As for suitability, I'm not against only showing a date picker for now.
It isn't perfect, but, it's more functionality than the user had previously.

I think I agree with @gautamsi that https://www.npmjs.com/package/chrono-node might be a better option; as if you only need date precision, why not use the calendarDay field?

@pnodet
Copy link
Contributor Author

pnodet commented Jan 29, 2024

As for suitability, I'm not against only showing a date picker for now.
It isn't perfect, but, it's more functionality than the user had previously.

For timestamps I think we could do a date picker first and another PR for timestamps if needed?

I think I agree with @gautamsi that https://www.npmjs.com/package/chrono-node might be a better option

I'm okay to update this PR to use the package but tbh I'm not quite sure I understand how this would look like. I mean what does the user input visually looks like if we were to use chrono-node?

if you only need date precision, why not use the calendarDay field?

I think with time we have moved away from calendarDay, finding them hard to use for broader usage in growing applications. Timestamps have shown to be more versatiles / flexible. But maybe it's a pre-conceived idea and time to reconsider. Are calendarDay filterable in a search page?

@pnodet
Copy link
Contributor Author

pnodet commented Mar 14, 2024

Hi @dcousens sorry to bother you! What are the blocking points for this PR? It's something we'd like to have in keystone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants