From 5400c81e29c50588fdfb7dd16628f2cbed0e9d2b Mon Sep 17 00:00:00 2001 From: John Cowen Date: Fri, 7 May 2021 12:07:11 +0100 Subject: [PATCH] ui: [BUGFIX] De-duplicate Tag rendering (#10186) * Add some tests for duplicated and non-duplicated tags * Ensure tags get de-duped and add docs * Update docs to include info on the recursive-ness --- .changelog/10186.txt | 3 + .../app/components/tag-list/README.mdx | 75 +++++++++++++++++++ .../app/components/tag-list/index.hbs | 45 ++++++----- .../app/components/tag-list/index.js | 5 -- .../acceptance/dc/services/show/tags.feature | 48 ++++++++++++ .../steps/dc/services/show/tags-steps.js | 10 +++ .../consul-ui/tests/pages/dc/services/show.js | 5 ++ 7 files changed, 168 insertions(+), 23 deletions(-) create mode 100644 .changelog/10186.txt create mode 100644 ui/packages/consul-ui/app/components/tag-list/README.mdx delete mode 100644 ui/packages/consul-ui/app/components/tag-list/index.js create mode 100644 ui/packages/consul-ui/tests/acceptance/dc/services/show/tags.feature create mode 100644 ui/packages/consul-ui/tests/acceptance/steps/dc/services/show/tags-steps.js diff --git a/.changelog/10186.txt b/.changelog/10186.txt new file mode 100644 index 0000000000..7922e332e7 --- /dev/null +++ b/.changelog/10186.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: De-duplicate tags in rendered tag listings +``` diff --git a/ui/packages/consul-ui/app/components/tag-list/README.mdx b/ui/packages/consul-ui/app/components/tag-list/README.mdx new file mode 100644 index 0000000000..1421cef7fa --- /dev/null +++ b/ui/packages/consul-ui/app/components/tag-list/README.mdx @@ -0,0 +1,75 @@ +# TagList + +Template only component for rendering a list of tags. You can pass either/or/and `@tags=tags` and/or `@item=item` (`item` must have a `Tags` property) for ease. If you pass both they are merged and de-duped. + +Tags are de-duplicated when rendered. + +```hbs preview-template + + +
+ + +``` + +`TagList` is also a recursive component, which can currently be used for when you need to wrap the `TagList` component in more DOM, but you only want that DOM to appear if your array of tags is non-empty. + + +```hbs preview-template +
+
+ This list has tags therefore the red border div will also be rendered. +
+ + +
+ +
+ + +
+ +
+ +
+
+ This list has no tags therefore the tags _and_ red border div will **not** be rendered. +
+ + +
+ +
+ + +
+``` + +## Arguments + +| Argument | Type | Default | Description | +| --- | --- | --- | --- | +| `item` | `Object` | | Object with a `Tags` property equalling an array of string tags | +| `tags` | `Array` | | An array of string tags | + +## Yields + +When used as a block level component the `TagList` yields itself, see above example for usage. + +| Property | Type | Description | +| --- | --- | --- | +| `Tags` | `Array` | The resulting collection of data after searching/sorting/filtering | diff --git a/ui/packages/consul-ui/app/components/tag-list/index.hbs b/ui/packages/consul-ui/app/components/tag-list/index.hbs index da39146b22..9927c13174 100644 --- a/ui/packages/consul-ui/app/components/tag-list/index.hbs +++ b/ui/packages/consul-ui/app/components/tag-list/index.hbs @@ -1,20 +1,29 @@ -{{#if (gt item.Tags.length 0)}} - {{#if (has-block)}} - {{yield - (component 'tag-list' item=item) +{{#let (union (or @item.Tags (array)) (or @tags (array))) as |tags|}} + {{#if (gt tags.length 0)}} + {{#if (has-block)}} + {{yield + (component 'tag-list' item=@item) + }} + {{else}} +
+
+ {{t "components.tag-list.title" + default=(array + "common.consul.tags" + ) }} - {{else}} -
-
- - Tags - -
-
- {{#each item.Tags as |item|}} - {{item}} - {{/each}} -
-
+
+
+ {{#each tags as |item|}} + {{item}} + {{/each}} +
+
+ {{/if}} {{/if}} -{{/if}} +{{/let}} diff --git a/ui/packages/consul-ui/app/components/tag-list/index.js b/ui/packages/consul-ui/app/components/tag-list/index.js deleted file mode 100644 index 4798652642..0000000000 --- a/ui/packages/consul-ui/app/components/tag-list/index.js +++ /dev/null @@ -1,5 +0,0 @@ -import Component from '@ember/component'; - -export default Component.extend({ - tagName: '', -}); diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/show/tags.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/show/tags.feature new file mode 100644 index 0000000000..1632ba1807 --- /dev/null +++ b/ui/packages/consul-ui/tests/acceptance/dc/services/show/tags.feature @@ -0,0 +1,48 @@ +@setupApplicationTest +Feature: dc / services / show / tags + Background: + Given 1 datacenter model with the value "dc1" + And 1 node models + Scenario: A service with multiple tags + Given 1 service model from yaml + --- + - Service: + Name: service + Kind: ~ + Tags: + - tag + - tag1 + - tag2 + --- + When I visit the service page for yaml + --- + dc: dc1 + service: service + --- + And I see tags on the tabs + When I click tags on the tabs + And I see tagsIsSelected on the tabs + And I see 3 tag models on the tabs.tagsTab component + Scenario: A service with multiple duplicated tags + Given 1 service model from yaml + --- + - Service: + Name: service + Kind: ~ + Tags: + - tag1 + - tag2 + - tag + - tag + - tag1 + - tag2 + --- + When I visit the service page for yaml + --- + dc: dc1 + service: service + --- + And I see tags on the tabs + When I click tags on the tabs + And I see tagsIsSelected on the tabs + And I see 3 tag models on the tabs.tagsTab component diff --git a/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show/tags-steps.js b/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show/tags-steps.js new file mode 100644 index 0000000000..3231912b98 --- /dev/null +++ b/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show/tags-steps.js @@ -0,0 +1,10 @@ +import steps from '../../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui/packages/consul-ui/tests/pages/dc/services/show.js b/ui/packages/consul-ui/tests/pages/dc/services/show.js index 7936ce8a43..883502b671 100644 --- a/ui/packages/consul-ui/tests/pages/dc/services/show.js +++ b/ui/packages/consul-ui/tests/pages/dc/services/show.js @@ -36,5 +36,10 @@ export default function(visitable, clickable, attribute, collection, text, inten name: text('[data-test-service-name]'), }), }; + page.tabs.tagsTab = { + tags: collection('.tag-list dd > span', { + name: text(), + }), + }; return page; }