Add presetted ranges to DateRange and DateTimeRange inputs #3781
Conversation
| import { react2angular } from 'react2angular'; | ||
| import DatePicker from 'antd/lib/date-picker'; | ||
| import { clientConfig } from '@/services/auth'; | ||
| import { Moment } from '@/components/proptypes'; | ||
|
|
||
| const { RangePicker } = DatePicker; | ||
|
|
||
| export const PRESETTED_RANGES = { |
There was a problem hiding this comment.
It seemed interesting to export here since Date Time Ranges will basically have the same Presetted ranges + its specific ones (with time included in the ranges).
There was a problem hiding this comment.
With this implementation the presets could go "stale" since they're calculated on page load (right?). Ideally should be calculated on preset click / calendar open. But it's unlikely so let's just keep it in mind.
There was a problem hiding this comment.
I can use the function notation instead to avoid this :)
|
@gabrieldutra this is really great! A few thoughts:
|
I tried to keep it with less than 2 rows for this reason. Unfortunately there's no easy way to implement the "more..." 😕. It would require to use Antd's
According to moment's docs: "As of version 2.1.0, moment#startOf('week') uses the locale aware week start day.". The default seems to be
There's indeed a chance it is confusing at first, but then it shows the specific days in the calendar and when you set to it, specific dates will show up. Dynamic settings are the second part of #3009, which I do have an idea to have a "Dynamic toggle" (should discuss this later in the issue). The general idea here is to first give value without much, if this turns out to be confusing we can just remove those and let them only in the "Dynamic" options. |
|
It's not considering work weekdays, but the whole week ^^ (I think this is what you expect when considering "This/Last week") |
|
Actually in this context, I think people expect a work week.
Maybe we will add a setting for "week starts on"? Taking this from local
computer might not always work: think of our team, for me and Ran it starts
on Sunday but for you it starts on Monday :)
…On Sat, May 11, 2019, 04:16 Gabriel Dutra ***@***.***> wrote:
Btw, why is en-US set to Sun-Sat when should actually be Mon-Fri?
It's not considering work weekdays, but the whole week ^^ (I think this is
what you expect when considering "This/Last week")
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3781 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAROLCYDFBT2Y4A5GY3C7LPUYM63ANCNFSM4HLUOF4A>
.
|
Can this be a point of conflict too 🤔 ? I've reached some friends that use Redash and this can be more confusing than I initially thought...
A conclusion is that this will depend a lot from each business model, but for this part this is not that relevant as this will be only a shorthand and people can learn from the dates that appear in there. So for this part the options I see are:
To this part I'm supporting 1 or 2. In case the setting option becomes relevant when doing Part 2 of #3009, we can extend it here as well. |
|
anything stopping us from merging this? |
I suggest to actually bundle these presets along with #3009 instead. |
|
Closing in favor of #3904 |
What type of PR is this? (check all applicable)
Description
This is related to the first part of #3009, it adds a few presetted options (from Antd presetted ranges) to DateRange and DateTimeRange inputs.
I tried to think the ranges that could be useful (and the ones suggested on the issue :)), thought add suggestions if you have any 🙂.
Related Tickets & Documents
#3009
Mobile & Desktop Screenshots/Recordings (if there are UI changes)