Add <DateFilter > component
#2849
Conversation
| .sort((a, b) => b[1]().getTime() - a[1]().getTime()); | ||
|
|
||
| const getSelectedOption = (options, value) => { | ||
| const selectedId = Array.isArray(value) ? value[0] : value; |
There was a problem hiding this comment.
decision Ideally, the option ID is enough to set the selected item. However, the value exposed on change is a couple [ID, date] and I thought it would be convenient to accept it as value as well otherwise it would require us to handle the difference in format when storing it in the local/global state.
| if ( | ||
| Array.isArray(option) && | ||
| typeof option[1] === 'function' && | ||
| option[1]() instanceof Date |
There was a problem hiding this comment.
decision Options come with a value function so that the actual reference date is calculated at selection time. That's simpler for the consumer as it already has a date, as expected from a date filter, which can just store the value and use it at its convenience.
That comes with a little drawback: if the date reflects a period (for example, 10 minutes ago), that value is never updated unless a new selection is made.
Another viable solution would be to return a number instead of a Date object; such a number can represent the milliseconds back from now. Being a relative value it does not have to update. However, the using component must handle the calculation itself.
There was a problem hiding this comment.
I am fine with the decision/tradeoff choice made.
There was a problem hiding this comment.
I don't see any major blocker. Eager to start using it in the activity log page 😄
A thought that came to my mind is about what gets send to the API call.
I believe the following format is what is going to be send and what the API accepts 2024-08-05T08:00:16.549046Z, @gagandeepb could you confirm, please?
question: should we be concerned about browser timezone vs server timezone? 🤔
| }; | ||
|
|
||
| export const WithCustomOptions = { | ||
| args: { |
There was a problem hiding this comment.
nitpick: since no title has been provided, the rendered story shows Filter undefined
| if ( | ||
| Array.isArray(option) && | ||
| typeof option[1] === 'function' && | ||
| option[1]() instanceof Date |
There was a problem hiding this comment.
I am fine with the decision/tradeoff choice made.
| * | ||
| * @component | ||
| * @example | ||
| * // <DateFilter |
There was a problem hiding this comment.
question: what we currently do is selecting a moment in the past (1h ago, etc...) and we want it to be the starting point of a timeframe within which we want to filter some resource (activity log at the moment).
DateFilter, is quite generic and might lead to thinking it is about selecting a specific date (which somehow happens under the hood).
What about a more specific name for the actual use case? Something around timeframe/timespan?
(TimeSpan is also something in ActivityLogsSettingsModal, tho, which is not the same thing, I reckon)
There was a problem hiding this comment.
might lead to thinking it is about selecting a specific date
It's exactly what happens. There are shortcuts, but also a date picker (which will be implemented in the next iteration)
There was a problem hiding this comment.
Ok, feel free to keep current name 👍
There was a problem hiding this comment.
If server assumes system/db time is UTC, the browser would/should do the same, I guess. All timestamps are assumed to be UTC, in ISO8601 format serialized as a string and expected in this format by the BE API. cc: @nelsonkopliku
@nelsonkopliku I think we shouldn't, the filter should refer to values as they come from the server. It will be up to the user to consider that he/she is looking at something generated into a server with its timezone. |
| }; | ||
|
|
||
| export const WithCustomOptions = { | ||
| args: { |
Description
Add component to filter some dates in the past. It can be configured with the options to show, although it has some defaults. Please take a look at the storybook for all the configurable options.
Every option is composed of a triple:
As a shortcut, an option can be specified as just its ID if it's available amongst the pre-configured ones/
The exposed interface comes with some assumptions, I'll leave comments in this PR pointing to the relevant code parts.
The component has also been added as a configurable filter in
<ComposedFilter />.