A major renaming: DashboardQuery -> DashboardState #6059
Conversation
RobertJoonas
force-pushed
the
fe-query-to-dashboardstate
branch
from
90de553 to
8d84d45
Compare
| * @param {string} config.interval - The interval of the dashboardState, e.g. `minute`, `hour` | ||
| * @param {boolean} config.longForm - Whether the formatted result should be in long or | ||
| * short form. | ||
| * @param {string} config.period - The period of the query, e.g. `12mo`, `day` | ||
| * @param {string} config.period - The period of the dashboardState, e.g. `12mo`, `day` |
There was a problem hiding this comment.
meganitpick, non-blocking: in this context, the original is more appropriate I think
| // Returns a combination of date and hour. This is because | ||
| // small intervals like hour may return multiple days | ||
| // depending on the query period. | ||
| // depending on the dashboardState period. |
There was a problem hiding this comment.
meganitpick, non-blocking: the original one is more appropriate inside the comment
|
apata
left a comment
apata
left a comment
There was a problem hiding this comment.
Wow, we do use the word query in a lot of places. Great job getting them all replaced successfully!
Regarding the new name, I can't think of a better one, but I think internally we should be careful with it for future features. Everything the user sees on the dashboard is part of its state, but not everything should be part of DashboardState and DashboardStateContext. Otherwise, we may trigger unnecessary re-renders that in turn make the page slow for the users.
2 participants
Changes
This PR is a pure refactor dealing only with renaming. The diff is quite enormous so I decided to open a separate PR just for that.
Renaming query -> dashboardState globally in the frontend makes sense because:
resolvedFilters(used for flattening segment filters so the FE could be aware of what filters are applied), andlabels(for rendering country, city, region, segment names in filter labels)StatsQueryobject is about to be introduced with [WIP] New internal stats endpoint #6035, so this is to avoid a naming clash.Tests
Changelog
Documentation
Dark mode