ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Keta Patel" <patel...@us.ibm.com>
Subject Re: Review Request 41577: AMBARI-11670: Width of services menu container is fixed and causes part of longer service names to be rendered outside the frame
Date Mon, 21 Dec 2015 23:54:20 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41577/
-----------------------------------------------------------

(Updated Dec. 21, 2015, 11:54 p.m.)


Review request for Ambari and Alexandr Antonenko.


Changes
-------

Hello Alexandr, thank you for the comments on the patch, I have made the required changes
and have uploaded a new patch.
1. I have removed the use of "!important" in the element's CSS style.
2. I have also removed the inline CSS style and have added new class in the application.less
file.
3. The "alertsCount" function is also updated as shown in the comment. The menu_item.hbs file
now makes use of the original single "if" condition to display alerts.


Bugs: AMBARI-11670
    https://issues.apache.org/jira/browse/AMBARI-11670


Repository: ambari


Description
-------

If a service has a really long name (19 characters or more), part of the service name is rendered
outside the frame. Any action icons for e.g. refresh also appear outside the frame.

The frame width should change dynamically to accommodate a longer service name or the service
name should wrap to the next line.


Diffs
-----

  ambari-web/app/assets/test/tests.js d13767f 
  ambari-web/app/templates/main/service/menu_item.hbs 8842858 
  ambari-web/app/views/main/service/menu.js e70dea2 
  ambari-web/test/views/main/service/menu_test.js PRE-CREATION 

Diff: https://reviews.apache.org/r/41577/diff/


Testing
-------

The cause of the long service name rendering outside the frame of the menu-container was because
the CSS style for the service names didn't allow word wrapping. While working on this issue,
we also realized that the refresh icon could be overlapped by the alert box when either the
service name was long or the alert count was big. 
We fixed these issues by surrounding a <div> block around the health icon, service name,
refresh icon and alert box. The width of all these 4 components are fixed to a suitable amount
so that all the 4 components now appear consistently in their respective positions. 
To allow for long service names, its CSS style is updated to allow for word-wrapping on white
space. For the case of long words in the service name that can't fit in one line in its <div>
block, we have added the CSS style to allow for word-breaks also.
In the case of alert box, the width of its <div> block allows alert counts having a
maximum of 2 digits to fit properly. An alert count of greater than 99 can overlap the refresh
icon. So to avoid this overlap, we display the alert count as "99+" for alert counts greater
than 99. The exact value of the alert count can be seen in the Summary page of the Service.
We came up with this fix considering that the average case of alert counts usually involves
2 digits or less. Any count greater than that is not very likely to occur and even if for
some reason there are a lot of alerts, then showing the exact count in the fixed width menu
container comes only secondary to showing that the Service has alerts.


To implement the logic of showing "99+" in the alert box, we added a new function "isAlertsCountBig"
in the ambari-web/app/views/main/service/menu.js. This function returns "true" when the "alertsCount"
is greater than 99, otherwise returns "false".

Test cases:
1. when "alertsCount"=0 (no alerts), "isAlertsCountBig" should return "false"
2. when "alertsCount"=5 (average case), "isAlertsCountBig" should return "false"
3. when "alertsCount"=200 (big alert count), "isAlertsCountBig" should return "true"
4. when "alertsCount"=99 (corner case of the condition), "isAlertsCountBig" should return
"false"


Ambari-web tests for the original trunk (without patch):

  11734 tests complete (14 seconds)
  137 tests pending

Ambari-web tests for the original trunk (after applying patch with 4 new tests):

  11738 tests complete (14 seconds)
  137 tests pending


File Attachments (updated)
----------------

AMBARI-11670: new patch
  https://reviews.apache.org/media/uploaded/files/2015/12/21/b3b1e648-4984-4433-a57a-f7f1c2349b22__AMBARI-11670.patch


Thanks,

Keta Patel


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message