You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/02/01 22:15:59 UTC

[GitHub] [incubator-pinot] JoeFuentes-WebDev commented on a change in pull request #6504: [TE]frontend - alert detail item UI fix

JoeFuentes-WebDev commented on a change in pull request #6504:
URL: https://github.com/apache/incubator-pinot/pull/6504#discussion_r568177195



##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,80 +1,104 @@
 <div class="te-search-results__header row">
   <div class="te-search-results__title-group col-md-10 col-xs-9">
     <div class="te-search-results__title">
-      <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</h3>
+      <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>
+        {{alertData.detectionName}}
+      </h3>
       {{#if (eq displayMode "list")}}
-      {{#link-to "manage.explore" alertData.id}}
-      <div class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</div>
-      {{/link-to}}
+        {{#link-to "manage.explore" alertData.id class="te-search-results__title-link"}}
+          <p class="te-search-results__title-name" title={{alertData.detectionName}}>
+            {{alertData.detectionName}}
+          </p>
+        {{/link-to}}
       {{/if}}
       {{#if isLoadError}}
-      <div
-        class="te-search-results__tag {{if (eq displayMode "list") "te-search-results__tag--list"}} {{if alertData.isActive "te-search-results__tag--active"}}">
-        Error
-      </div>
+        <div class="te-search-results__tag
+             {{if (eq displayMode " list") "te-search-results__tag--list"}}
+             {{if alertData.isActive "te-search-results__tag--active"}}">
+          Error
+        </div>
       {{else}}
-      <div
-        class="te-search-results__tag {{if (eq displayMode "list") "te-search-results__tag--list"}} {{if alertData.isActive "te-search-results__tag--active"}}">
-        {{if alertData.isActive "Active" "Inactive"}}
-      </div>
-      {{#x-toggle
+        <div class="te-search-results__tag
+             {{if (eq displayMode " list") "te-search-results__tag--list"}}
+             {{if alertData.isActive "te-search-results__tag--active"}}">
+          {{if alertData.isActive "Active" "Inactive"}}
+        </div>
+        {{#x-toggle
           value=alertData.isActive
           classNames="te-toggle te-toggle--form te-toggle--left report-toggle activation-toggle"
           theme="ios"
           id="active-toggle"
           showLabels=false
           name="activeToggle"
-      onToggle=(action "toggleAlertActivation")
-      as |toggle|}}
-      {{toggle.switch theme='ios' onLabel='diff on' offLabel='diff off'}}
-      {{/x-toggle}}
+          onToggle=(action "toggleAlertActivation") as |toggle|
+        }}
+          {{toggle.switch theme="ios" onLabel="diff on" offLabel="diff off"}}
+        {{/x-toggle}}
       {{/if}}
     </div>
   </div>
   <div class="col-md-2 col-xs-3">
-    <div class="pull-right">{{yield}}</div>
+    <div class="pull-right">
+      {{yield}}
+    </div>
   </div>
 </div>
-<h4 class="te-alert-page__subtitle">{{alertData.description}}</h4>
+<h4 class="te-alert-page__subtitle">
+  {{alertData.description}}
+</h4>
 {{#if (not isLoadError)}}
-<div class="te-alert-detail-breakdown row">
-  <dl class="col-xs-12 col-sm-5 row">
-    <dt class="col-md-4">Metric</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.metric}}>
-      {{if alertData.metric alertData.metric 'N/A'}}
-    </dd>
-
-    <dt class="col-md-4">Dataset</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.dataset}}>
-      {{if alertData.dataset alertData.dataset 'N/A'}}
-    </dd>
-
-    <dt class="col-md-4">Filtered By</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.filters}}>
-      {{if alertData.filters alertData.filters 'N/A'}}</dd>
-
-    <dt class="col-md-4">Breakdown By</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.dimensionExploration}}>
-      {{if alertData.dimensionExploration alertData.dimensionExploration 'N/A'}}</dd>
-  </dl>
-
-  <dl class="col-xs-12 col-sm-5 row">
-    <dt class="col-md-4">Created By</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.createdBy}}>
-      {{if alertData.createdBy alertData.createdBy 'N/A'}}</dd>
-
-    <dt class="col-md-4">Updated By</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.updatedBy}}>
-      {{if alertData.updatedBy alertData.updatedBy 'N/A'}}</dd>
-
-    <dt class="col-md-4">Last detection</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.lastDetectionTime}}>
-      {{if alertData.lastDetectionTime alertData.lastDetectionTime 'N/A'}}</dd>
-
-    <dt class="col-md-4">Subscribed Groups</dt>
-    <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{subscribedGroups}}>
-      {{if subscribedGroups subscribedGroups 'N/A'}}
-    </dd>
-  </dl>
-</div>
-{{/if}}
+  <div class="te-alert-detail-breakdown row">
+    <dl class="col-xs-12 col-sm-5 row">
+      <dt class="col-md-4">
+        Metric
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.metric}}>
+        {{if alertData.metric alertData.metric "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Dataset
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.dataset}}>
+        {{if alertData.dataset alertData.dataset "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Filtered By
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.filters}}>
+        {{if alertData.filters alertData.filters "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Breakdown By
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.dimensionExploration}}>
+        {{if alertData.dimensionExploration alertData.dimensionExploration "N/A"}}
+      </dd>
+    </dl>
+    <dl class="col-xs-12 col-sm-5 row">
+      <dt class="col-md-4">
+        Created By
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.createdBy}}>
+        {{if alertData.createdBy alertData.createdBy "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Updated By
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.updatedBy}}>
+        {{if alertData.updatedBy alertData.updatedBy "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Last detection
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.lastDetectionTime}}>
+        {{if alertData.lastDetectionTime alertData.lastDetectionTime "N/A"}}
+      </dd>
+      <dt class="col-md-4">
+        Subscribed Groups
+      </dt>
+      <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{subscribedGroups}}>
+        {{if subscribedGroups subscribedGroups "N/A"}}
+      </dd>
+    </dl>
+  </div>
+{{/if}}

Review comment:
       I realize this may be beyond the scope, but...   could we do something like:  
   
   ```
   {{#each this.alertDataArray as |data|}}
      <dt class="col-md-4">
           {{data.name}}
         </dt>
         <dd class="col-md-8 te-search-results__value{{valueClassSuffix}}" title={{alertData.lastDetectionTime}}>
           {{if data.value data.value  "N/A"}}
         </dd>
     {{/each}}
   ```
   
   where **_alertDataArray_** is and array build inside the component/controller?  
   
   if we want to be even more efficient... how about creating a small component 
   
   Again - I realize it's way beyond the scope, so feel free to skip. I just wanted to bring it up so we can keep it in mind as we refactor the codebase
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org