fix(nimbus): hide weekly metrics from displaying on results page popout card if not available #14261
Conversation
moibra05
requested review from
freshstrangemusic,
jaredlockhart and
yashikakhurana
as code owners
…ut card if not available
yashikakhurana
left a comment
yashikakhurana
left a comment
There was a problem hiding this comment.
Good point, nice work adding the condition
…ut card if not available (#14261) Because - The current new results page ui assumes that all metrics contain weekly data which is not true. This results in empty "Weekly breakdown" sections that serve no purpose. This commit - Conditionally renders the "Weekly breakdown" component if the specific metric supports weekly results Fixes #14257
| if weekly_data: | ||
| weekly_metric_data[metric_metadata["slug"]] = weekly_data | ||
|
|
There was a problem hiding this comment.
you know what you can also add a property like is weekly_data_available and then renders accordingly, just a suggestion
There was a problem hiding this comment.
yes i originally thought to do something like this but figured that since id only be using that property in a single area it might be simpler to just not include weekly data if it exists so that i can just use a simple if weekly_metric_data check and not have to worry about additional flags or default values
There was a problem hiding this comment.
for the maintainability purpose, would suggest to have the property as it helps other to easily work with it in the future, otherwise have to dig into templates and logic to understand it
Because
This commit
Fixes #14257