From 655f244f1b6e594d7d9816d80cbf15164b5c0e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sun, 28 Apr 2019 17:32:39 +0100 Subject: [PATCH] refactor(ui): use button instead of a badge for filter elements --- ui/src/App.scss | 3 + ui/src/Common/Colors.js | 42 +++++++----- ui/src/Components/Labels/BaseLabel/index.js | 28 ++++---- ui/src/Components/Labels/BaseLabel/index.scss | 21 ++++++ .../Components/Labels/BaseLabel/index.test.js | 66 ++++++++++--------- .../Labels/FilterInputLabel/index.css | 34 ---------- .../Labels/FilterInputLabel/index.js | 38 +++++------ .../Labels/FilterInputLabel/index.scss | 45 +++++++++++++ .../Labels/FilterInputLabel/index.test.js | 26 ++++---- .../__snapshots__/index.test.js.snap | 2 +- .../Components/NavBar/FilterInput/index.css | 9 +++ ui/src/Components/NavBar/FilterInput/index.js | 2 +- 12 files changed, 189 insertions(+), 127 deletions(-) delete mode 100644 ui/src/Components/Labels/FilterInputLabel/index.css create mode 100644 ui/src/Components/Labels/FilterInputLabel/index.scss diff --git a/ui/src/App.scss b/ui/src/App.scss index a222b3ad7..0766396b3 100644 --- a/ui/src/App.scss +++ b/ui/src/App.scss @@ -38,3 +38,6 @@ $nav-tabs-link-active-bg: #fff; .cursor-pointer { cursor: pointer; } +.cursor-text { + cursor: text; +} diff --git a/ui/src/Common/Colors.js b/ui/src/Common/Colors.js index 3ca90caf1..9f71a2e37 100644 --- a/ui/src/Common/Colors.js +++ b/ui/src/Common/Colors.js @@ -1,17 +1,7 @@ // fallback class for labels -const DefaultLabelClass = "badge-warning components-label-dark"; - -// labels configured as static will have badge-${this class} -const StaticColorLabelClass = "badge-info components-label-dark"; - -// alertname label will use this one -const AlertNameLabelClass = "badge-dark components-label-dark"; - -// alert state label will use one of those, based on the value -const StateLabelClassMap = Object.freeze({ - active: "badge-danger components-label-dark", - suppressed: "badge-success components-label-dark", - unprocessed: "badge-secondary components-label-bright" +const DefaultLabelClassMap = Object.freeze({ + badge: "badge-warning components-label-dark", + btn: "btn-warning components-label-dark" }); // same but for borders @@ -27,11 +17,27 @@ const BackgroundClassMap = Object.freeze({ unprocessed: "bg-secondary" }); +const StaticColorLabelClassMap = Object.freeze({ + badge: "badge-info components-label-dark", + btn: "btn-info components-label-dark" +}); + +const AlertNameLabelClassMap = Object.freeze({ + badge: "badge-dark components-label-dark", + btn: "btn-dark components-label-dark" +}); + +const StateLabelClassMap = Object.freeze({ + active: "danger", + suppressed: "success", + unprocessed: "secondary" +}); + export { - DefaultLabelClass, - StaticColorLabelClass, - AlertNameLabelClass, - StateLabelClassMap, + DefaultLabelClassMap, + StaticColorLabelClassMap, + AlertNameLabelClassMap, BorderClassMap, - BackgroundClassMap + BackgroundClassMap, + StateLabelClassMap }; diff --git a/ui/src/Components/Labels/BaseLabel/index.js b/ui/src/Components/Labels/BaseLabel/index.js index 560f905db..7b6f024a4 100644 --- a/ui/src/Components/Labels/BaseLabel/index.js +++ b/ui/src/Components/Labels/BaseLabel/index.js @@ -3,10 +3,10 @@ import PropTypes from "prop-types"; import { AlertStore } from "Stores/AlertStore"; import { - StaticColorLabelClass, - StateLabelClassMap, - DefaultLabelClass, - AlertNameLabelClass + StaticColorLabelClassMap, + DefaultLabelClassMap, + AlertNameLabelClassMap, + StateLabelClassMap } from "Common/Colors"; import { QueryOperators, FormatQuery, StaticLabels } from "Common/Query"; @@ -22,15 +22,17 @@ class BaseLabel extends Component { value: PropTypes.string.isRequired }; - getClassAndStyle(name, value, extraClass = "") { + getClassAndStyle(name, value, extraClass, baseClass) { const { alertStore } = this.props; + const elementType = baseClass || "badge"; + const data = { style: {}, className: "", baseClassNames: [ "components-label", - "badge", + elementType, "text-nowrap", "text-truncate", "mw-100" @@ -39,11 +41,15 @@ class BaseLabel extends Component { }; if (name === StaticLabels.AlertName) { - data.colorClassNames.push(AlertNameLabelClass); + data.colorClassNames.push(AlertNameLabelClassMap[elementType]); } else if (name === StaticLabels.State) { - data.colorClassNames.push(StateLabelClassMap[value] || DefaultLabelClass); + data.colorClassNames.push( + StateLabelClassMap[value] + ? `${elementType}-${StateLabelClassMap[value]}` + : DefaultLabelClassMap[elementType] + ); } else if (alertStore.settings.values.staticColorLabels.includes(name)) { - data.colorClassNames.push(StaticColorLabelClass); + data.colorClassNames.push(StaticColorLabelClassMap[elementType]); } else { const c = alertStore.data.getColorData(name, value); if (c) { @@ -62,12 +68,12 @@ class BaseLabel extends Component { ); } else { // if not fall back to class - data.colorClassNames.push(DefaultLabelClass); + data.colorClassNames.push(DefaultLabelClassMap[elementType]); } } data.className = `${[...data.baseClassNames, ...data.colorClassNames].join( " " - )} ${extraClass}`; + )} ${extraClass || ""}`; return data; } diff --git a/ui/src/Components/Labels/BaseLabel/index.scss b/ui/src/Components/Labels/BaseLabel/index.scss index 2bb6bc87e..996c6cd2c 100644 --- a/ui/src/Components/Labels/BaseLabel/index.scss +++ b/ui/src/Components/Labels/BaseLabel/index.scss @@ -12,24 +12,45 @@ .components-label-bright { color: $black; + &:hover { + color: $black; + } + &.components-label-name, .components-label-name { color: lighten($black, 20%); + &:hover { + color: lighten($black, 20%); + } } &.components-label-value, .components-label-value { color: $black; + &:hover { + color: $black; + } } } .components-label-dark { color: $white; + &:hover { + color: $white; + } + &.components-label-name, .components-label-name { color: darken($white, 10%); + &:hover { + color: darken($white, 10%); + } } + &.components-label-value, .components-label-value { color: $white; + &:hover { + color: $white; + } } } diff --git a/ui/src/Components/Labels/BaseLabel/index.test.js b/ui/src/Components/Labels/BaseLabel/index.test.js index a0db5f8db..8b24359fc 100644 --- a/ui/src/Components/Labels/BaseLabel/index.test.js +++ b/ui/src/Components/Labels/BaseLabel/index.test.js @@ -4,9 +4,9 @@ import { shallow } from "enzyme"; import { AlertStore } from "Stores/AlertStore"; import { - StaticColorLabelClass, - DefaultLabelClass, - AlertNameLabelClass, + StaticColorLabelClassMap, + DefaultLabelClassMap, + AlertNameLabelClassMap, StateLabelClassMap } from "Common/Colors"; import { BaseLabel } from "."; @@ -36,62 +36,68 @@ const FakeBaseLabel = (name = "foo", value = "bar") => { }; describe("", () => { - it("static label uses StaticColorLabelClass", () => { + it("static label uses StaticColorLabelClassMap.badge", () => { alertStore.settings.values.staticColorLabels = ["foo", "job", "bar"]; const tree = FakeBaseLabel(); - expect(tree.find(".components-label").hasClass(StaticColorLabelClass)).toBe( - true - ); + expect( + tree.find(".components-label").hasClass(StaticColorLabelClassMap.badge) + ).toBe(true); }); - it("non-static label doesn't use StaticColorLabelClass", () => { - alertStore.settings.values.staticColorLabels = []; + Object.entries(StaticColorLabelClassMap).map(([key, val]) => + it(`non-static label doesn't use StaticColorLabelClassMap.${key}`, () => { + alertStore.settings.values.staticColorLabels = []; + const tree = FakeBaseLabel(); + expect(tree.find(".components-label").hasClass(val)).toBe(false); + }) + ); + + it("label with no special color information should use DefaultLabelClassMap.badge", () => { const tree = FakeBaseLabel(); - expect(tree.find(".components-label").hasClass(StaticColorLabelClass)).toBe( - false - ); + expect( + tree.find(".components-label").hasClass(DefaultLabelClassMap.badge) + ).toBe(true); }); - it("label with no special color information should use DefaultLabelClass", () => { - const tree = FakeBaseLabel(); - expect(tree.find(".components-label").hasClass(DefaultLabelClass)).toBe( - true - ); - }); - - it("alertname label should use AlertNameLabelClass", () => { + it("alertname label should use AlertNameLabelClassMap.badge", () => { const tree = FakeBaseLabel("alertname", "foo"); - expect(tree.find(".components-label").hasClass(AlertNameLabelClass)).toBe( - true - ); + expect( + tree.find(".components-label").hasClass(AlertNameLabelClassMap.badge) + ).toBe(true); }); it("@state=active label should use StateLabelClassMap.active class", () => { const tree = FakeBaseLabel("@state", "active"); expect( - tree.find(".components-label").hasClass(StateLabelClassMap.active) + tree + .find(".components-label") + .hasClass(`badge-${StateLabelClassMap.active}`) ).toBe(true); }); it("@state=suppressed label should use StateLabelClassMap.suppressed class", () => { const tree = FakeBaseLabel("@state", "suppressed"); expect( - tree.find(".components-label").hasClass(StateLabelClassMap.suppressed) + tree + .find(".components-label") + .hasClass(`badge-${StateLabelClassMap.suppressed}`) ).toBe(true); }); it("@state=unprocessed label should use StateLabelClassMap.unprocessed class", () => { const tree = FakeBaseLabel("@state", "unprocessed"); expect( - tree.find(".components-label").hasClass(StateLabelClassMap.unprocessed) + tree + .find(".components-label") + .hasClass(`badge-${StateLabelClassMap.unprocessed}`) ).toBe(true); }); - it("@state with unknown label should use DefaultLabelClass", () => { + it("@state with unknown label should use DefaultLabelClassMap.badge", () => { const tree = FakeBaseLabel("@state", "foobar"); - expect(tree.find(".components-label").hasClass(DefaultLabelClass)).toBe( - true - ); + expect( + tree.find(".components-label").hasClass(DefaultLabelClassMap.badge) + ).toBe(true); }); it("style prop on a label included in staticColorLabels should be empty", () => { diff --git a/ui/src/Components/Labels/FilterInputLabel/index.css b/ui/src/Components/Labels/FilterInputLabel/index.css deleted file mode 100644 index 1213fd947..000000000 --- a/ui/src/Components/Labels/FilterInputLabel/index.css +++ /dev/null @@ -1,34 +0,0 @@ -.badge.components-filteredinputlabel { - /* make fonts bigger */ - font-size: 90%; - - /* fix align after text-truncate */ - vertical-align: middle; - - padding-top: 0.15rem; - padding-bottom: 0.15rem; - padding-left: 0.15rem; - - min-height: 1.5rem; - - /* ensure there's some space between filters - if there are multiple rows in the input */ - margin-top: 1px; - margin-bottom: 1px; -} - -.badge.components-filteredinputlabel > .badge, -.badge.components-filteredinputlabel > svg { - /* match outer badge font size */ - font-size: 90%; - line-height: 100%; - - /* we have badge inside a badge, make the inner one a little brighter - in case both use same color classes */ - filter: brightness(115%); -} - -.badge.components-filteredinputlabel > .close { - /* make close button bigger, to match font size of the badge */ - font-size: 100%; -} diff --git a/ui/src/Components/Labels/FilterInputLabel/index.js b/ui/src/Components/Labels/FilterInputLabel/index.js index db5e7f479..490266f39 100644 --- a/ui/src/Components/Labels/FilterInputLabel/index.js +++ b/ui/src/Components/Labels/FilterInputLabel/index.js @@ -14,7 +14,7 @@ import { QueryOperators } from "Common/Query"; import { TooltipWrapper } from "Components/TooltipWrapper"; import { BaseLabel } from "Components/Labels/BaseLabel"; -import "./index.css"; +import "./index.scss"; const FilterInputLabel = observer( class FilterInputLabel extends BaseLabel { @@ -48,7 +48,8 @@ const FilterInputLabel = observer( let cs = this.getClassAndStyle( filter.matcher === QueryOperators.Equal ? filter.name : "", filter.matcher === QueryOperators.Equal ? filter.value : "", - "components-filteredinputlabel" + "components-filteredinputlabel btn-sm border-0", + "btn" ); const showCounter = @@ -59,13 +60,14 @@ const FilterInputLabel = observer( const rootClasses = filter.applied ? cs.className : [ - "badge-secondary components-filteredinputlabel", + "btn-secondary btn-sm border-0 components-filteredinputlabel", ...cs.baseClassNames ].join(" "); return ( - {filter.isValid ? ( @@ -76,21 +78,20 @@ const FilterInputLabel = observer( ) : null ) : ( - - - + ) ) : ( - - - + )} - - + + ); } } diff --git a/ui/src/Components/Labels/FilterInputLabel/index.scss b/ui/src/Components/Labels/FilterInputLabel/index.scss new file mode 100644 index 000000000..e2368ecfd --- /dev/null +++ b/ui/src/Components/Labels/FilterInputLabel/index.scss @@ -0,0 +1,45 @@ +@import "~bootswatch/dist/flatly/variables"; + +.components-filteredinputlabel > .badge, +.components-filteredinputlabel > svg { + /* match outer badge font size */ + font-size: 100%; + + /* we have badge inside a badge, make the inner one a little brighter + in case both use same color classes */ + filter: brightness(115%); +} + +button.btn.components-filteredinputlabel { + cursor: default; + margin-top: 0.125rem; + margin-bottom: 0.125rem; +} + +button.components-label.btn { + &.btn-primary:hover { + background-color: $primary; + } + &.btn-secondary:hover { + background-color: $secondary; + } + &.btn-success:hover { + background-color: $success; + } + &.btn-info:hover { + background-color: $info; + } + &.btn-warning:hover { + background-color: $warning; + } + &.btn-danger:hover { + background-color: $danger; + } + &.btn-light:hover { + background-color: $light; + } + &.btn-dark:hover { + // same as in App.scss + background-color: #3b4247; + } +} diff --git a/ui/src/Components/Labels/FilterInputLabel/index.test.js b/ui/src/Components/Labels/FilterInputLabel/index.test.js index 6bfdf6587..b8827684a 100644 --- a/ui/src/Components/Labels/FilterInputLabel/index.test.js +++ b/ui/src/Components/Labels/FilterInputLabel/index.test.js @@ -54,35 +54,35 @@ const ValidateOnChange = newRaw => { }; describe(" className", () => { - it("unapplied filter with '=' matcher should use 'badge-secondary' class", () => { - ValidateClass("=", false, "badge-secondary"); + it("unapplied filter with '=' matcher should use 'btn-secondary' class", () => { + ValidateClass("=", false, "btn-secondary"); }); - it("unapplied filter with any matcher other than '=' should use 'badge-secondary' class", () => { + it("unapplied filter with any matcher other than '=' should use 'btn-secondary' class", () => { for (const matcher of NonEqualMatchers) { - ValidateClass(matcher, false, "badge-secondary"); + ValidateClass(matcher, false, "btn-secondary"); } }); - it("applied filter with '=' matcher and no color should use 'badge-warning' class", () => { - ValidateClass("=", true, "badge-warning"); + it("applied filter with '=' matcher and no color should use 'btn-warning' class", () => { + ValidateClass("=", true, "btn-warning"); }); - it("applied filter with any matcher other than '=' and no color should use 'badge-warning' class", () => { + it("applied filter with any matcher other than '=' and no color should use 'btn-warning' class", () => { for (const matcher of NonEqualMatchers) { - ValidateClass(matcher, true, "badge-warning"); + ValidateClass(matcher, true, "btn-warning"); } }); - it("applied filter included in staticColorLabels with '=' matcher should use 'badge-info' class", () => { + it("applied filter included in staticColorLabels with '=' matcher should use 'btn-info' class", () => { alertStore.settings.values.staticColorLabels = ["foo"]; - ValidateClass("=", true, "badge-info"); + ValidateClass("=", true, "btn-info"); }); - it("applied filter included in staticColorLabels with any matcher other than '=' should use 'badge-warning' class", () => { + it("applied filter included in staticColorLabels with any matcher other than '=' should use 'btn-warning' class", () => { alertStore.settings.values.staticColorLabels = ["foo"]; for (const matcher of NonEqualMatchers) { - ValidateClass(matcher, true, "badge-warning"); + ValidateClass(matcher, true, "btn-warning"); } }); }); @@ -171,7 +171,7 @@ describe(" onChange", () => { filter={alertStore.filters.values[0]} /> ); - const button = tree.find("button"); + const button = tree.find(".close"); button.simulate("click"); expect(alertStore.filters.values).toHaveLength(1); expect(alertStore.filters.values).toContainEqual( diff --git a/ui/src/Components/NavBar/FilterInput/__snapshots__/index.test.js.snap b/ui/src/Components/NavBar/FilterInput/__snapshots__/index.test.js.snap index 619697e12..95dc37fe4 100644 --- a/ui/src/Components/NavBar/FilterInput/__snapshots__/index.test.js.snap +++ b/ui/src/Components/NavBar/FilterInput/__snapshots__/index.test.js.snap @@ -21,7 +21,7 @@ exports[` matches snapshot on default render 1`] = ` -
+
{ this.onInputClick(this.inputStore.ref, event); }}