Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Data catalog UI: MVP #5628
base: main
Are you sure you want to change the base?
Data catalog UI: MVP #5628
Changes from 32 commits
c2a27bc
fc71e17
6489e64
13f9cca
f256f5b
8d6a25b
58df678
2b3e67e
72a4e87
7eaabcb
dd40ca0
a279c2e
cfedd93
b0ca9bb
f0d3ca1
ecafa9d
c0900c7
056b3ab
d068f50
0297e41
8453d8b
670e206
482d662
4e53afa
fcf6b21
81789af
4162b70
da3c337
92f1a40
41d69c3
4b5ed9b
f1ad82e
a155e57
eb9f3be
cbe1901
934de8b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this makes it specific to Result Status? Would this be better as a more generically named component, or does it need specific properties required with more strict type? (eg.
colorScheme
which has to be ofSTATUS_COLOR_MAP
enum type). You might consider exporting theSTATUS_COLOR_MAP
enum from this component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout-- deleted this component and changed all of its uses to use the more generic BadgeCell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this file should be renamed/moved since it's specific to TableV2 cells
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this component just seems superfluous. It's either too specifically named (where the label could be another prop), or not specific enough about the children it accepts. Does it always need to be inside a form or can it be moved out to the "common" parent directory?