feat(explore-attr-breakdowns): Adding new loading state UI #103953
Conversation
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx
Outdated
Show resolved
Hide resolved
nsdeschenes
left a comment
nsdeschenes
left a comment
There was a problem hiding this comment.
One small nit to look at
| import Placeholder from 'sentry/components/placeholder'; | ||
| import {IconMegaphone} from 'sentry/icons/iconMegaphone'; | ||
| import {t} from 'sentry/locale'; | ||
| import {space} from 'sentry/styles/space'; |
There was a problem hiding this comment.
Small nit: Are you able to utilize the theme.space.<size> for the spaces over this util function 👀
Abdkhan14
merged commit 10e2130
into
abdk/attr-breakdowns-search-persistence
| const SelectionHintContainer = styled('div')` | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: ${space(0.5)}; | ||
| margin-bottom: ${space(1)}; | ||
| `; |
There was a problem hiding this comment.
it would be really cool if, while touching those styled components, we could refactor them towards the layout primitives of the design-system.
For example, this one would just be a simple:
<Flex orientation="column" gap="xs" align="center">
| <ChartsGrid> | ||
| {Array.from({length: 9}).map((_, index) => ( | ||
| <LoadingChart key={index} /> | ||
| ))} |
There was a problem hiding this comment.
Bug: Loading skeleton shows 9 charts, actual content shows 12
The LoadingCharts component renders 9 placeholder charts (Array.from({length: 9})), but the actual content views use CHARTS_PER_PAGE = CHARTS_COLUMN_COUNT * 4 which equals 12 charts per page (3 columns × 4 rows vs 3 columns × 3 rows). This mismatch causes a visual layout jump when loading completes, as the grid height changes from 3 rows to potentially 4 rows.
Uh oh!
There was an error while loading. Please reload this page.