ACCUMULO-4764 Move html from js to templates#342
Conversation
|
Can one of the admins verify this patch? |
ctubbsii
left a comment
ctubbsii
left a comment
There was a problem hiding this comment.
+1; still room for further improvement, but nothing I see that needs to change here before this is applied.
| --> | ||
| <div><h3>${title}</h3></div> | ||
| <div id="gcBanner"></div> | ||
| <div id="gcBanner" style="display: none;"><div class="alert alert-danger" role="alert">Collector is Unavailable</div></div> |
There was a problem hiding this comment.
This could be simplified by adding the id="gcBanner" to the div tag that's already there for the alert tags. Also, best practice is to not put inline style attributes. Could instead put a rule for #gcBanner { display: none; } in the main stylesheet for the site.
There was a problem hiding this comment.
Fixed. It turns out these style tags were not needed.
| <tr><td class="left" style="display: table-cell;"><a href="/tables">Tables</a></td><td class="right" style="display: table-cell;"></td></tr> | ||
| <tr><td class="left" style="display: table-cell;"><a href="/tservers">Tablet Servers</a></td><td class="right" style="display: table-cell;"></td></tr> | ||
| <tr><td class="left" style="display: table-cell;"><a href="/tservers">Dead Tablet Servers</a></td><td class="right" style="display: table-cell;"></td></tr> | ||
| <tr><td class="left" style="display: table-cell;">Tablets</td><td class="right" style="display: table-cell;"></td></tr> |
There was a problem hiding this comment.
This is another place where inline styles could be replaced with a separate globally defined class or id CSS rule.
There was a problem hiding this comment.
Fixed. It turns out these style tags were not needed.
|
|
||
| <caption><span class="table-caption">Scan Status</span><br/></caption> | ||
| <tbody><tr><th class="firstcell" onclick="sortTable(0)">Server </th> | ||
| <th onclick="sortTable(1)" title="Number of scans presently running"># </th> |
There was a problem hiding this comment.
Might be able to change sortTable implementation to automatically detect which column number to sort on. May have to pass sortTable(this), but probably not. This isn't important, but just an idea for an improvement, since the numbers could result in copy/paste errors if the columns are reordered or new columns are added and the parameters aren't updated. Making the method call resilient in the face of column reordering or adding, would be nice in the future.
There was a problem hiding this comment.
Created a newbie ticket as I thought this would be a good ticket for newcomers since it only requires knowledge of javascript to complete.
|
Thanks for the reviews. I am going to make a few more similar changes that I missed and I will push an update soon. |
4 participants
No description provided.