Skip to content
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

app-catalog: Fix UI bugs #150

Merged
merged 9 commits into from
Feb 11, 2025
Merged

app-catalog: Fix UI bugs #150

merged 9 commits into from
Feb 11, 2025

Conversation

skoeva
Copy link
Contributor

@skoeva skoeva commented Jan 23, 2025

These changes address minor bugs in the app catalog UI.

Fixes: #149

@skoeva skoeva added bug Something isn't working app-catalog labels Jan 23, 2025
@skoeva skoeva self-assigned this Jan 23, 2025
@skoeva skoeva linked an issue Jan 23, 2025 that may be closed by this pull request
@skoeva skoeva mentioned this pull request Jan 24, 2025
@skoeva skoeva marked this pull request as ready for review January 24, 2025 17:14
@skoeva skoeva requested a review from sniok January 24, 2025 17:14
@sniok
Copy link
Contributor

sniok commented Jan 27, 2025

Thanks for taking a look into this!

The install button turns light-blue on hover, let's keep it consistent with other black buttons we have in the app.

image

For example here's how cluster chooser button looks while hovered

image


When typing into the search the whole page freezes up, it sends a request and rerenders everything on each key press

CPU profiler shows this

image

I'd suggest adding some form debouncing to search state updates. Alternatively you could try useDeferredValue hook, although usually for this case simple debouncing works a bit better


I know this wasn't in the original issue but I've noticed that sometimes images are missing, looking at the url there's an undefined at the end of the path so maybe it wouldn't be hard to not show image at all when it's missing

image

@skoeva skoeva force-pushed the ui-app-catalog branch 2 times, most recently from 3d4f2f1 to e9e1080 Compare January 27, 2025 16:42
@skoeva
Copy link
Contributor Author

skoeva commented Jan 27, 2025

@sniok thanks for the input here, added those changes to the PR 😃

Copy link
Contributor

@sniok sniok 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, thanks!

@sniok
Copy link
Contributor

sniok commented Feb 7, 2025

@skoeva let's rebase and I'll merge this

@skoeva skoeva force-pushed the ui-app-catalog branch 3 times, most recently from 56cee9d to a8bad8e Compare February 7, 2025 13:43
@skoeva
Copy link
Contributor Author

skoeva commented Feb 7, 2025

@sniok added an additional commit because the app would crash without setting the params, but should be good now

@sniok sniok merged commit c9039fb into main Feb 11, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-catalog bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

app-catalog: UI papercuts
2 participants