From e4f076f81302fb9a466a0012de441cff692bca60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Fri, 19 Mar 2021 12:20:04 +0000
Subject: [PATCH] [UI] Fix small issues generating console errors (#8622)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Checkbox should use onChange, not onClick

This fixes react console errors:

You provided a `checked` prop to a form field without an `onChange` handler. This will render a read-only field. If the field should be mutable use `defaultChecked`. Otherwise, set either `onChange` or `readOnly`.

Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>

* Correctly pass key in metrics exporer

Instead of passing metric variable we pass 'metric' string, which causes console errors due to duplicated keys.

Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>

* Update tests

Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>
---
 .../react-app/src/pages/alerts/AlertContents.test.tsx  | 10 ++++++----
 web/ui/react-app/src/pages/alerts/AlertContents.tsx    |  8 ++------
 .../alerts/__snapshots__/AlertContents.test.tsx.snap   |  8 ++++----
 web/ui/react-app/src/pages/graph/MetricsExplorer.tsx   |  2 +-
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/web/ui/react-app/src/pages/alerts/AlertContents.test.tsx b/web/ui/react-app/src/pages/alerts/AlertContents.test.tsx
index e0832f99f..403cad6be 100644
--- a/web/ui/react-app/src/pages/alerts/AlertContents.test.tsx
+++ b/web/ui/react-app/src/pages/alerts/AlertContents.test.tsx
@@ -23,17 +23,19 @@ describe('AlertsContent', () => {
     { selector: '#firing-toggler', propName: 'firing' },
   ].forEach(testCase => {
     it(`toggles the ${testCase.propName} checkbox from true to false when clicked and back to true when clicked again`, () => {
-      wrapper.find(testCase.selector).invoke('onClick')(testCase.propName);
+      expect(wrapper.find(testCase.selector).prop('checked')).toBe(true);
+      wrapper.find(testCase.selector).simulate('change', { target: { checked: false } });
       expect(wrapper.find(testCase.selector).prop('checked')).toBe(false);
-      wrapper.find(testCase.selector).invoke('onClick')(testCase.propName);
+      wrapper.find(testCase.selector).simulate('change', { target: { checked: true } });
       expect(wrapper.find(testCase.selector).prop('checked')).toBe(true);
     });
   });
 
   it('toggles the "annotations" checkbox from false to true when clicked and back to false when clicked again', () => {
-    wrapper.find('#show-annotations-toggler').invoke('onClick')();
+    expect(wrapper.find('#show-annotations-toggler').prop('checked')).toBe(false);
+    wrapper.find('#show-annotations-toggler').simulate('change', { target: { checked: true } });
     expect(wrapper.find('#show-annotations-toggler').prop('checked')).toBe(true);
-    wrapper.find('#show-annotations-toggler').invoke('onClick')();
+    wrapper.find('#show-annotations-toggler').simulate('change', { target: { checked: false } });
     expect(wrapper.find('#show-annotations-toggler').prop('checked')).toBe(false);
   });
 });
diff --git a/web/ui/react-app/src/pages/alerts/AlertContents.tsx b/web/ui/react-app/src/pages/alerts/AlertContents.tsx
index b77f7575a..99d4c9688 100644
--- a/web/ui/react-app/src/pages/alerts/AlertContents.tsx
+++ b/web/ui/react-app/src/pages/alerts/AlertContents.tsx
@@ -55,10 +55,6 @@ const AlertsContent: FC<AlertsProps> = ({ groups = [], statsCount }) => {
     });
   };
 
-  const toggleAnnotations = () => {
-    setShowAnnotations({ checked: !showAnnotations.checked });
-  };
-
   return (
     <>
       <div className="d-flex togglers-wrapper">
@@ -69,7 +65,7 @@ const AlertsContent: FC<AlertsProps> = ({ groups = [], statsCount }) => {
               wrapperStyles={{ marginRight: 10 }}
               checked={filter[state]}
               id={`${state}-toggler`}
-              onClick={toggleFilter(state)}
+              onChange={toggleFilter(state)}
             >
               <Badge color={color} className="text-capitalize">
                 {state} ({statsCount[state]})
@@ -81,7 +77,7 @@ const AlertsContent: FC<AlertsProps> = ({ groups = [], statsCount }) => {
           wrapperStyles={{ marginLeft: 'auto' }}
           checked={showAnnotations.checked}
           id="show-annotations-toggler"
-          onClick={() => toggleAnnotations()}
+          onChange={({ target }) => setShowAnnotations({ checked: target.checked })}
         >
           <span style={{ fontSize: '0.9rem', lineHeight: 1.9 }}>Show annotations</span>
         </Checkbox>
diff --git a/web/ui/react-app/src/pages/alerts/__snapshots__/AlertContents.test.tsx.snap b/web/ui/react-app/src/pages/alerts/__snapshots__/AlertContents.test.tsx.snap
index fdbba2c9f..307a724c4 100644
--- a/web/ui/react-app/src/pages/alerts/__snapshots__/AlertContents.test.tsx.snap
+++ b/web/ui/react-app/src/pages/alerts/__snapshots__/AlertContents.test.tsx.snap
@@ -9,7 +9,7 @@ exports[`AlertsContent matches a snapshot 1`] = `
       checked={true}
       id="inactive-toggler"
       key="inactive"
-      onClick={[Function]}
+      onChange={[Function]}
       wrapperStyles={
         Object {
           "marginRight": 10,
@@ -32,7 +32,7 @@ exports[`AlertsContent matches a snapshot 1`] = `
       checked={true}
       id="pending-toggler"
       key="pending"
-      onClick={[Function]}
+      onChange={[Function]}
       wrapperStyles={
         Object {
           "marginRight": 10,
@@ -55,7 +55,7 @@ exports[`AlertsContent matches a snapshot 1`] = `
       checked={true}
       id="firing-toggler"
       key="firing"
-      onClick={[Function]}
+      onChange={[Function]}
       wrapperStyles={
         Object {
           "marginRight": 10,
@@ -77,7 +77,7 @@ exports[`AlertsContent matches a snapshot 1`] = `
     <Memo(Checkbox)
       checked={false}
       id="show-annotations-toggler"
-      onClick={[Function]}
+      onChange={[Function]}
       wrapperStyles={
         Object {
           "marginLeft": "auto",
diff --git a/web/ui/react-app/src/pages/graph/MetricsExplorer.tsx b/web/ui/react-app/src/pages/graph/MetricsExplorer.tsx
index 325cbfe66..363bfb3f6 100644
--- a/web/ui/react-app/src/pages/graph/MetricsExplorer.tsx
+++ b/web/ui/react-app/src/pages/graph/MetricsExplorer.tsx
@@ -25,7 +25,7 @@ class MetricsExplorer extends Component<Props, {}> {
         <ModalHeader toggle={this.toggle}>Metrics Explorer</ModalHeader>
         <ModalBody>
           {this.props.metrics.map(metric => (
-            <p className="metric" key="metric" onClick={this.handleMetricClick.bind(this, metric)}>
+            <p key={metric} className="metric" onClick={this.handleMetricClick.bind(this, metric)}>
               {metric}
             </p>
           ))}