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

Custom date range selector #66

Open
goenning opened this issue Dec 16, 2023 · 11 comments
Open

Custom date range selector #66

goenning opened this issue Dec 16, 2023 · 11 comments
Labels
feature request New feature or request

Comments

@goenning
Copy link
Member

Instead of predefined ranges, let the user select what they want

@goenning goenning added the feature request New feature or request label Dec 16, 2023
@prajwalkulkarni
Copy link
Contributor

Do you mean to filter and show the events based on a custom date range on the dashboard?

@goenning
Copy link
Member Author

Exactly, using something like: https://ui.shadcn.com/docs/components/calendar

It would support existing pre-defined ranges, but if the user wants a custom range they could use the calendar to pick a start/end date.

This would involve changing the backend and frontend, as the backend expects keys like: "7d", "30d", etc, instead of start-end date.

Are you interested in contributing to this issue as well?

@prajwalkulkarni
Copy link
Contributor

So, we shall be using components from shadcn here, right? As there is no DatePicker component in radix-ui that is provided out of the box. I came across this repo https://github.com/johnpolacek/date-range-picker-for-shadcn that I could try implementing. Does that sound good?
And is the backend written in C#? I don't have any experience in C# but if you could guide me here on what needs to be changed and where, it'd be helpful.

@goenning
Copy link
Member Author

There's a DatePicker on shadcn, I think we should use that one: https://ui.shadcn.com/docs/components/date-picker

Yes, backend is in C#. Syntax is very similar to TypeScript, so you won't have much trouble with it :)

I think we should implement this features in two steps:

  1. Change the Backend to accept a startDate, endDate and granularity parameters instead of hardcoded ranges. Then change the UI to send these dates when the user selects one of the existing ranges.
  2. Add a date picker for startDate and endDate so the user can select custom ranges instead of only the predefined ones.

What you think?

@prajwalkulkarni
Copy link
Contributor

Right, so the link that I shared above is built on top of the shadcn's DatePicker component which is more intuitive and easy to use.
I implemented the same by tweaking it a little to suit our needs, and here's how it looks. There's no data shown in the dashboard as I had updated the Parse method's parameters at the time of recording without fully gluing it to the frontend.

custom-date-picker.mp4

I believe the component can be further modified to include the "All time" option if needed.
I've started to make changes in the backend and I'm creating a WIP PR, I shall keep you updated as I progress :)

@goenning
Copy link
Member Author

That's looking great, better than I'd have thought, thank you @prajwalkulkarni

Some feedback:

  1. Yes, support for All Time would be great
  2. What's that "Compare" section on top?
  3. Check how it looks on mobile, it can be tricky to do this on small devices
  4. I'm using tabler-icons instead of lucide-react, they have very similar components, so just need to find a similar one
  5. For granularity, I think we should have a toggle component similar to this one (screenshot), and maybe we start with Day/Month only?
Screenshot 2024-02-21 at 20 20 00

🚀

@prajwalkulkarni
Copy link
Contributor

Hey @goenning need some input on how should we store and process the date input, since we are moving away from the predefined keys how should we send the selection range to the backend? My initial approach was to use two query params from and to and send the serialized date. However, this would require significant changes to be done both on frontend and the backend. Instead, we could use the existing period query param and send the range in unix timestamp format with - delimiter, Later this input can be again split into from and to on the delimiter, converted to date format and pass it to the Parse method, but I'm not sure if this is a good approach, could you kindly give some heads up here?

What's that "Compare" section on top?

It is actually a prop when passed shows the "vs" string between the two date ranges. I'm not passing it here so it is not visible however the "Compare" is still seen, we can remove it.

Check how it looks on mobile, it can be tricky to do this on small devices

Yeah, I will look into it.

@goenning
Copy link
Member Author

I think it would be best to have the from and to parameters.

Eventually this will become a public API, and it'll be easier for developers to use these parameters as it's fairly standard to have a from/to parameters.

If you do a new Date().toISOString() and send that to C#, it should fit into a DateTime parameter automatically. Did you have other issues with it?

That's why I suggested splitting this feature in 2 PRs. First change the backend/frontend to use from/to, while keeping the predefined keys only in frontend, then separate add the calendar UI which should then be a UI-only PR. What you think?

@prajwalkulkarni
Copy link
Contributor

prajwalkulkarni commented Feb 28, 2024

Eventually this will become a public API, and it'll be easier for developers to use these parameters as it's fairly standard to have a from/to parameters.

Fair enough.

If you do a new Date().toISOString() and send that to C#, it should fit into a DateTime parameter automatically. Did you have other issues with it?

I did try with new Date().toJSON() which gives a similar output, but for some reason, it didn't seem to work, I was a little overwhelmed with making lots of changes at once so didn't pay attention to what was causing the issue haha. Let me check on this again.

That's why I suggested splitting this feature in 2 PRs. First change the backend/frontend to use from/to, while keeping the predefined keys only in frontend, then separate add the calendar UI which should then be a UI-only PR.

Yeah, I think that is a good idea. I shall begin making changes on the backend first and then make UI changes as a separate PR.

@prajwalkulkarni
Copy link
Contributor

So I started by making the changes and looks like we'll have to update the parameter type of period or replace it with a new parameter of an external dependency @aptabase/web, how do we go about it?

@goenning
Copy link
Member Author

goenning commented Mar 2, 2024

I think we should replace period with startDate, endDate and granularity

The period param internally simply converts to those 3 parameters anyway

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

No branches or pull requests

2 participants