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

fix(Catalog): Using labels causes unintended navigation #1078

Merged
merged 1 commit into from
May 13, 2024

Conversation

lordrip
Copy link
Member

@lordrip lordrip commented May 13, 2024

Context

Currently, we're using the numLabels property of the LabelGroup component to show just a few Labels.

When the user clicks on the n more label, a navigation is performed and the UI is reloaded, causing the VSCode extension to fail.

This happens because LabelGroup is a child of the Form component, causing to execute the default Form behavior when selecting a Label

Changes

The fix is to move the Label bar outside of the Form component.

Screenshot

Before After
image image

fix: #1073
fix: #1074
fix: #911

Context:
Currently, we're using the `numLabels` property of the `LabelGroup` component to show just a few `Labels`.

When the user clicks on the `n more` label, a navigation is performed and the UI is reloaded, causing the VSCode extension to fail.

This happens because `LabelGroup` is a child of the `Form` component, causing to execute the default Form behavior when selecting a `Label`

Changes:
The fix is to move the Label bar outside of the `Form` component.

fix: KaotoIO#1073
fix: KaotoIO#1074
Copy link
Contributor

@lhein lhein left a comment

Choose a reason for hiding this comment

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

lgtm

@lordrip lordrip merged commit 649987a into KaotoIO:main May 13, 2024
8 checks passed
@lordrip lordrip deleted the fix/catalog-label-navigation branch May 13, 2024 11:15
Copy link
Member

@apupier apupier left a comment

Choose a reason for hiding this comment

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

would have been interesting to split the work between the label alignment and the filter category issue.
it would allow:

  • clearer review
  • easier history
  • easier to revert one or the other in case there are detected issues introduced

@lordrip
Copy link
Member Author

lordrip commented May 13, 2024

@apupier, I agree with you, but in this case, the fix for the category issue broke the styling of the filter bar, so I ended up with two scenarios:

  1. Fix the now broken style and redo it again to fix the other issue (present since long time ago 😢)
  2. Fix the styles taking into account the existing issue and fix them together

I decided for the later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants