-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
New querying horizontal UI - continuation #4239
Conversation
@samwinslow can you help me with a review when you can? wanted to do a checkpoint before the PR becomes too big. |
} | ||
} | ||
|
||
const showDateFilter = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering -- why are some of these flag getters objects, and some others are functions? Should we prefer one approach or the other? (I personally prefer the functions with switch
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm that's a good question, not sure actually (it's code quite old), but I guess that we have the functions for logic that requires checking other variables and just plain constants for when it's just a static definition. Will keep in mind for a refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! It's definitely nice to see TrendTabHorizontal as its own file.
Good catch on the mobile-side, I still need to optimize for mobile. Will do that in a subsequent PR. |
Failing test is unrelated (was failing before), merging. |
Changes
This PR continues work towards #4050 (issue still WIP, but working through bite-sized PRs). This PR handles most UI layout changes.
In addition:
InsightDisplayConfig.tsx
to be able to return different components in the new UI.TrendTabHorizontal.tsx
is introduced as a high-level component to encompass most changes to the layout.Checklist