👻 Refactor tags table to remove deprecated legacy table dep #1806
Conversation
ibolton336
force-pushed
the
tech-debt-legacy-tags
branch
from
ce9b406
to
4e27ffb
Compare
ibolton336
force-pushed
the
tech-debt-legacy-tags
branch
from
1c9cbda
to
601cb3b
Compare
Signed-off-by: Ian Bolton <ibolton@redhat.com>
ibolton336
force-pushed
the
tech-debt-legacy-tags
branch
from
601cb3b
to
65c93c9
Compare
| <Button | ||
| type="button" | ||
| id="create-tag-category" | ||
| aria-label="Create tag category" |
There was a problem hiding this comment.
the label is not translated
There was a problem hiding this comment.
To be snarky and pedantic -- it's not externalized
Signed-off-by: Ian Bolton <ibolton@redhat.com>
ibolton336
force-pushed
the
tech-debt-legacy-tags
branch
from
06e40ed
to
fc8fb99
Compare
| const tagNames = item?.tags | ||
| ?.map((tag) => tag.name) | ||
| .concat(tagCategoryNames) | ||
| .join(""); |
There was a problem hiding this comment.
Oddly this bit always annoyed me. Probably should be .join("^") or some name invalid character so the oddball false positive matches won't happen. "RedItchy" will match "ditch" but "Red^Itchy" will not.
| }, | ||
| { | ||
| title: item.tags ? item.tags.length : 0, | ||
| selectOptions: dedupeFunction( |
There was a problem hiding this comment.
The name filter list should contain what? Tag Category names AND all Tag Names? That's a bit confusing. Maybe have the Name filter look like the Tag filter on the application table? Maybe that is work for another PR?
Maybe something like this:
Column name: Tag Category
Filter 1: Tag Category (select list)
Filter 2: Tag (select list like Tags on Applications)
| {tagCategory.rank} | ||
| </Td> | ||
| <Td width={10} {...getTdProps({ columnKey: "color" })}> | ||
| <Color hex={categoryColor} /> |
There was a problem hiding this comment.
Something odd with this component such that it doesn't line up with the other text in the row.
There was a problem hiding this comment.
but also not a regression -- so that can be a followup PR as well
| <Td>{cell.title}</Td> | ||
| ))} | ||
| {(tagCategory.tags || []) | ||
| .sort((a, b) => a.name.localeCompare(b.name)) |
There was a problem hiding this comment.
Another place to use localeNumericCompare()
FWIW, localeNumbericCompare() could be changed a bit to have the locale be defaulted to "i18n.language" and ignore that part here...(could also be another PR)
/**
* Uses native string localCompare method with numeric option enabled.
*
* @param locale to be used by string compareFn
*/
export const localeNumericCompare = (
a: string,
b: string,
locale: string = i18n.language
): number => a.localeCompare(b, locale, { numeric: true });There was a problem hiding this comment.
I vote for a small nice PR!
ibolton336
added
the
cherry-pick/release-0.3
…#1806) Working towards resolving: konveyor#1283 --------- Signed-off-by: Ian Bolton <ibolton@redhat.com>
Working towards resolving:
#1283
Resolves https://issues.redhat.com/browse/MTA-2224