Add tags from filter_categories#206
Add tags from filter_categories#206drewbutlerbb4 wants to merge 3 commits intoclaimed-framework:mainfrom drewbutlerbb4:filter-tags
Conversation
Signed-off-by: Andrew-Butler <Andrew.Butler@ibm.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: drewbutlerbb4 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@ckadner the default for tags is to not have any tags if you think there should be a default tag I can add one. |
| const description = firstSentence(props.description || 'Component for your Pipelines.') | ||
|
|
||
| const tags = tag.includes(',') ? tag.split(',') : [tag] | ||
| const tags = Object.keys(asset.filter_categories).map((key: any) => asset.filter_categories[key]) |
There was a problem hiding this comment.
does this work if the object has no filter_categories? i.e. pipelines don't have it
There was a problem hiding this comment.
Ah I thought we made the filter_categories tag mandatory for all assets, but I forgot that pipelines are different. I'll fix that.
There was a problem hiding this comment.
@drewbutlerbb4 -- could you modify the code to allow for missing filter_categories so we can merge this?
There was a problem hiding this comment.
@ckadner -- This is an unfinished draft, in a meeting we discussed this and decided it was not ready to merge. With the addition of a variable number and length of tags, the individual cards can get bugged. Long tags can cause portions of the card to become invisible this is especially noticeable for Models (which have a logo for the model type).
If your opinion since the meeting has changed then I can fix to allow for missing filter_categories and merge this. But we will need to add a follow-up issue for the card bug.
There was a problem hiding this comment.
This PR comment was about a functional change to make sure the code works in the absence of the filter_categories for pipelines.
My take-away from earlier meetings was, that this PR was to wait for other priorities at the time, and to limit the number of filter_categories label values to 3 (or 5) based on experiments.
In the future I (/we) should add conclusions from meetings to the issues or pull requests right away to avoid confusions.
There was a problem hiding this comment.
@ckadner, sure I have updated as you have requested.
no, no default tags |
Signed-off-by: Andrew-Butler <Andrew.Butler@ibm.com>
Signed-off-by: Andrew-Butler <Andrew.Butler@ibm.com>
3 participants
Adds the tags from asset.filter_categories into the Featured Page cards.
Signed-off-by: Andrew-Butler Andrew.Butler@ibm.com