rusackas commented on PR #41527:
URL: https://github.com/apache/superset/pull/41527#issuecomment-5099507889

   Thanks for this, the sub-hour presets and the get_since_until hour-regex fix 
are a nice, well-tested improvement. 
   
   One thing before I can dig into review mode though: the diff in 
CommonFrame.tsx does a lot more than the description covers. It adds a whole 
new 'Other' custom N/unit picker with its own state management, which isn't 
mentioned in the summary or screenshots. Was that intentional scope, or did it 
creep in? If it's wanted, I'd split it into its own PR so it gets proper design 
attention... or at least build the rationale into the PR description/title. If 
not, let's trim this back to just the four presets. 
   
   Also worth a quick prettier pass locally since pre-commit is currently 
failing on formatting.


-- 
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]

Reply via email to