----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41577/#review111456 -----------------------------------------------------------
ambari-web/app/templates/main/service/menu_item.hbs (line 22) <https://reviews.apache.org/r/41577/#comment171658> Try not to use "!important", as this makes very hard to maintain in future ambari-web/app/templates/main/service/menu_item.hbs (line 29) <https://reviews.apache.org/r/41577/#comment171657> avoid using inline styles in template, use class and css file for this purpose ambari-web/app/views/main/service/menu.js (line 73) <https://reviews.apache.org/r/41577/#comment171656> There is no need to create this property (isAlertsCountBig) and additional "if" in templete, you can simply do it in ambari-web/app/views/main/service/menu.js by changing alertsCount : alertsCount: function () { return this.get('content.alertsCount') >99 ? "99+" : this.get('content.alertsCount') ; }.property('content.alertsCount'), - Alexandr Antonenko On Dec. 19, 2015, 1:34 a.m., Keta Patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41577/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2015, 1:34 a.m.) > > > Review request for Ambari and Alexandr Antonenko. > > > 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 > > > Thanks, > > Keta Patel > >
