Skip to content

ref(tags): use placeholder loader#113025

Merged
JonasBa merged 3 commits into
masterfrom
jb/tags/DE-1110
Apr 15, 2026
Merged

ref(tags): use placeholder loader#113025
JonasBa merged 3 commits into
masterfrom
jb/tags/DE-1110

Conversation

@JonasBa
Copy link
Copy Markdown
Member

@JonasBa JonasBa commented Apr 15, 2026

Replace incorrectly used loadingIndicator with Placeholder component

…chemaHintsList

Swap the LoadingIndicator for a row of Placeholder skeleton pills that
match the shape and layout of the actual tag hint buttons. The loading
container uses overflow:hidden so excess pills are clipped cleanly.

Co-Authored-By: Claude <noreply@anthropic.com>
@JonasBa JonasBa requested a review from a team April 15, 2026 08:56
@JonasBa JonasBa requested a review from a team as a code owner April 15, 2026 08:56
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 15, 2026

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 15, 2026
<Flex justify="center" align="center" height="24px">
<LoadingIndicator mini />
</Flex>
<SchemaHintsContainer aria-label={t('Schema Hints List')} style={{overflow: 'hidden'}}>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried seeing if we can convert to Container or Flex, but this uses > * {flex-shrink: 0}, so I'd rather not touch it

@sentry
Copy link
Copy Markdown
Contributor

sentry Bot commented Apr 15, 2026

Sentry Snapshot Testing

Name Added Removed Modified Renamed Unchanged Status
sentry-frontend
sentry-frontend
0 0 0 0 204 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

Copy link
Copy Markdown
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! I was wondering why we always see the placeholder even when there’s cached data, and I found that we bind isLoading to isFetching, which is true for all fetches, including background refetches:

not sure what the idea behind this is but it feels wrong? Also I had to navigate 5 layers to get to that code so not sure what the impact of changing this one thing would be :/

@nsdeschenes
Copy link
Copy Markdown
Contributor

looks good! I was wondering why we always see the placeholder even when there’s cached data, and I found that we bind isLoading to isFetching, which is true for all fetches, including background refetches:

not sure what the idea behind this is but it feels wrong? Also I had to navigate 5 layers to get to that code so not sure what the impact of changing this one thing would be :/

I'll be doing some refactoring around this, iirc we do want this to stay as isFetching from testing around with it.

@JonasBa JonasBa enabled auto-merge (squash) April 15, 2026 13:52
@JonasBa JonasBa merged commit 17a4dad into master Apr 15, 2026
65 checks passed
@JonasBa JonasBa deleted the jb/tags/DE-1110 branch April 15, 2026 13:59
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants