sfirke commented on issue #26651: URL: https://github.com/apache/superset/issues/26651#issuecomment-6345994481
Thanks @CodeWithPravinMaske for laying out the options so clearly, and for being willing to rework this. Here's where I've landed after reading back through this thread and the PR discussion. Where things stand: this SIP started in 2024 asking for an "alternative" time filter. @andy-clapson's revision (Part 1) instead proposes adding a Calendar choice to the existing time range control. #44527 went the original way and added a separate filter_date_range type. I would prefer we avoid a second date filter type. Adding a new filter type gives dashboard owners another choice to make, gives viewers a different widget from one dashboard to the next, and would need an alias or migration if we merge them later. **Proposal**: rework #44527 as your option 3. Keep a single Time range native filter and add a setting to it, something like Picker style: Full / Calendar. - **Full** is today's behavior, unchanged. Existing filters (no setting saved) stay on Full. - **Calendar**: the filter bar shows only your inline date-range picker. No popover, no Range type dropdown. Picking dates gives a fixed range with the end date included, and clearing it means no filter. - The owner still sets the default value with the full control, so a default like "Last week" still loads the dashboard with that filter, displaying its actual dates. Then how those display to the end user depends on whether they've picked Full or Calendar. - Edge case: if the default includes a time of day (e.g. "10 days before now" currently uses the current time), the calendar shouldn't imply whole days that aren't actually included. Rounding to include the entire day, and/or a small text hint, both seem reasonable to me. Maybe the small text hint, to preserve the current behavior? I think the text hint would be for the dashboard editor setting the filter - then they can use a different filter option to get complete dates - and for the end user. - Should new filters default to Calendar or Full? I'm not sure. I think on my instance I'd want the default to be Calendar. But maybe this could be a config setting so admins can specify? If we make it a config setting we don't need to debate which default. I would imagine some tech-focused orgs would want the Full as default. This drops the new plugin folder, the `filter_date_range` constant, and the separate reports branch. Your `parse/format` utils, the exclusive-end (+1 day) handling, and their tests all carry over. **Design for reuse**: please build the calendar as a shared component, e.g. alongside the `DateFilterControl` frames, not inside a filter plugin. SIP Part 1 (the Calendar choice inside the time range box, which Explore and the dashboard filter share) can then reuse it as-is, so both places behave the same way: same end-date rule, same display, and eventually the same quick-range buttons. That makes "Part 1" above a small follow-up once this lands. **Then, Part 2**: the groundwork for letting extensions add their own range types (fiscal calendars, week numbers, etc.) by making the list of range types extensible instead of hard-coded. That's separate work after Part 1. @andy-clapson, would you be amenable to adding this Full vs. Calendar filter-bar setting to the SIP text (as a Part 1a, say)? I'm not asking you to even type anything above, don't want to make more work for you, I just want to get all the brains that care about this issue agreed on what it is we're doing. If you'd be good with the reworked-#44527 as the first part of this SIP, followed by Part 1 and Part 2, then I think we could call a vote on this. Pravin, what do you think about my proposed rework for your open PR? Thanks again for driving this forward after it stalled for so long. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
