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 2020/10/29 03:00:28 UTC
[GitHub] [incubator-pinot] abachuk opened a new pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
abachuk opened a new pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210
## Description
- Changing UI alignment and formatting on the Alert page according to the latest design mock.
- Updating eslintrc to include a single quote rule
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] jihaozh merged pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
jihaozh merged pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] abachuk commented on a change in pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
abachuk commented on a change in pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210#discussion_r514405365
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,22 +1,24 @@
<div class="te-search-results__header">
<div class="te-search-results__title-group">
<div class="te-search-results__title">
- <span title={{alertData.detectionName}}>{{alertData.detectionName}}</span>
+ <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</h3>
Review comment:
the class name doesn't hint the truncated text (there are no handlers), instead, the browser is using the title tag to display native tooltip on hover regardless if the text is truncated or not)
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] zhangloo333 commented on a change in pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
zhangloo333 commented on a change in pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210#discussion_r513904629
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,103 +1,79 @@
-<div class="te-search-results__header">
- <div class="te-search-results__title-group">
+<div class="te-search-results__header row">
+ <div class=" te-search-results__title-group col-md-10 col-xs-9">
Review comment:
The indent issue before `te-search-results`
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,22 +1,24 @@
<div class="te-search-results__header">
<div class="te-search-results__title-group">
<div class="te-search-results__title">
- <span title={{alertData.detectionName}}>{{alertData.detectionName}}</span>
+ <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</h3>
Review comment:
it's weird to see the title attribute in `h` tag. If we want to support the screenreader, we may set a rule to add all title attribute for h tag.
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] abachuk commented on a change in pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
abachuk commented on a change in pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210#discussion_r514378050
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,22 +1,24 @@
<div class="te-search-results__header">
<div class="te-search-results__title-group">
<div class="te-search-results__title">
- <span title={{alertData.detectionName}}>{{alertData.detectionName}}</span>
+ <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</h3>
Review comment:
In this case, it's used for displaying the full title on hover when it's truncated.
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] abachuk commented on a change in pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
abachuk commented on a change in pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210#discussion_r514379194
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,103 +1,79 @@
-<div class="te-search-results__header">
- <div class="te-search-results__title-group">
+<div class="te-search-results__header row">
+ <div class=" te-search-results__title-group col-md-10 col-xs-9">
Review comment:
good catch, fixed.
----------------------------------------------------------------
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
[GitHub] [incubator-pinot] zhangloo333 commented on a change in pull request #6210: [TE] frontend - abachuk/alert-details-ui-tweaks - formatting alert header
Posted by GitBox <gi...@apache.org>.
zhangloo333 commented on a change in pull request #6210:
URL: https://github.com/apache/incubator-pinot/pull/6210#discussion_r514393658
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/self-serve-alert-yaml-details/template.hbs
##########
@@ -1,22 +1,24 @@
<div class="te-search-results__header">
<div class="te-search-results__title-group">
<div class="te-search-results__title">
- <span title={{alertData.detectionName}}>{{alertData.detectionName}}</span>
+ <h3 class="te-search-results__title-name" title={{alertData.detectionName}}>{{alertData.detectionName}}</h3>
Review comment:
That's great. I curious before why you use the same `alertData.detectionName` twice in the tag of text and title field. If the class name can hint truncated, It would better to use `te-search-results__title-name--truncated`. When users read HTML code, they will easy to know the usage.
----------------------------------------------------------------
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