<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0" /> <base href="https://issues.jboss.org" />
<title>Message Title</title>
</head>
<body class="jira" style="color: #333; font-family: Arial, sans-serif; font-size: 14px; line-height: 1.429">
<table id="background-table" cellpadding="0" cellspacing="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; background-color: #f5f5f5; border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<!-- header here -->
<tr>
<td id="header-pattern-container" style="padding: 0px; border-collapse: collapse; padding: 10px 20px">
<table id="header-pattern" cellspacing="0" cellpadding="0" border="0" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="header-avatar-image-container" valign="top" style="padding: 0px; border-collapse: collapse; vertical-align: top; width: 32px; padding-right: 8px"> <img id="header-avatar-image" class="image_fix" src="https://community.jboss.org/gravatar/1ce36ce1c246b989f9cd79504c400b08?d=mm&s=48" height="32" width="32" border="0" style="border-radius: 3px; vertical-align: top" />
</td>
<td id="header-text-container" valign="middle" style="padding: 0px; border-collapse: collapse; vertical-align: middle; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 1px"> <a class="user-hover" rel="lfryc" id="email_lfryc" href="https://issues.jboss.org/secure/ViewProfile.jspa?name=lfryc" style="color:#3b73af;; color: #3b73af; text-decoration: none">Lukáš Fryč</a> <strong>updated</strong> an issue
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td id="email-content-container" style="padding: 0px; border-collapse: collapse; padding: 0 20px">
<table id="email-content-table" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; border-spacing: 0; border-collapse: separate">
<tr>
<!-- there needs to be content in the cell for it to render in some clients -->
<td class="email-content-rounded-top mobile-expand" style="padding: 0px; border-collapse: collapse; color: #fff; padding: 0 15px 0 16px; height: 15px; background-color: #fff; border-left: 1px solid #ccc; border-top: 1px solid #ccc; border-right: 1px solid #ccc; border-bottom: 0; border-top-right-radius: 5px; border-top-left-radius: 5px; height: 10px; line-height: 10px; padding: 0 15px 0 16px; mso-line-height-rule: exactly">
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand " style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff">
<table class="page-title-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td class="page-title-pattern-first-line " style="padding: 0px; border-collapse: collapse; font-family: Arial, sans-serif; font-size: 14px; padding-top: 10px"> <a href="https://issues.jboss.org/browse/AGPUSH" style="color: #3b73af; text-decoration: none">AeroGear Push</a> / <a href="https://issues.jboss.org/browse/AGPUSH-695" style="color: #3b73af; text-decoration: none"><img src="cid:jira-generated-image-static-health-87f3446c-e278-4038-8e78-2d9ac5b4e094" height="16" width="16" border="0" align="absmiddle" alt="Enhancement" style="vertical-align: text-bottom" /></a> <a href="https://issues.jboss.org/browse/AGPUSH-695" style="color: #3b73af; text-decoration: none">AGPUSH-695</a>
</td>
</tr>
<tr>
<td style="vertical-align: top;; padding: 0px; border-collapse: collapse; padding-right: 5px; font-size: 20px; line-height: 30px; mso-line-height-rule: exactly" class="page-title-pattern-header-container"> <span class="page-title-pattern-header" style="font-family: Arial, sans-serif; padding: 0; font-size: 20px; line-height: 30px; mso-text-raise: 2px; mso-line-height-rule: exactly; vertical-align: middle"> <a href="https://issues.jboss.org/browse/AGPUSH-695" style="color: #3b73af; text-decoration: none">Admin UI: apply findings of Code Style review</a> </span>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand wrapper-special-margin" style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff; padding-top: 10px; padding-bottom: 5px">
<table class="keyvalue-table" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<th style="color: #707070; font: normal 14px/20px Arial, sans-serif; text-align: left; vertical-align: top; padding: 2px 0">
Change By:
</th>
<td style="padding: 0px; border-collapse: collapse; font: normal 14px/20px Arial, sans-serif; padding: 2px 0 2px 5px; vertical-align: top"> <a class="user-hover" rel="lfryc" id="email_lfryc" href="https://issues.jboss.org/secure/ViewProfile.jspa?name=lfryc" style="color:#3b73af;; color: #3b73af; text-decoration: none">Lukáš Fryč</a>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand issue-description-container" style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff; padding-top: 5px; padding-bottom: 10px">
<table class="text-paragraph-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 2px">
<tr>
<td class="text-paragraph-pattern-container mobile-resize-text " style="padding: 0px; border-collapse: collapse; padding: 0 0 10px 0"> <span class="diffcontext">Forum discussion:<br />http://aerogear-dev.1069024.n5.nabble.com/aerogear-dev-AG-Push-Console-review-td8004.html<br /><br />----<br /><br /></span> <span class="diffremovedchars" style="background-color: #ffe7e7; text-decoration:line-through;">*</span> <span class="diffaddedchars" style="background-color:#ddfade;">{quote}<br /></span> <span class="diffcontext"> The name of your angularjs module is "newadminApp". I think it should<br />be something like "agconsole".<br /></span> <span class="diffaddedchars" style="background-color:#ddfade;">{quote}</span> <span class="diffcontext"><br /></span> <span class="diffremovedchars" style="background-color: #ffe7e7; text-decoration:line-through;">*</span> <span class="diffaddedchars" style="background-color:#ddfade;"><br />created AGPUSH-719<br /><br />{quote}<br /></span> <span class="diffcontext"> You're missing feedback for user on many places. If the error says<br />'Something went wrong...', it doesn't say much to a user. Even changing<br />it to denote the current operation would be better, something like<br />'Unable to create new application'. On most of the $resource.* methods<br />you're missing the feedback, too. For most of them the information about<br />success would be annoying, but maybe it could be interesting for user to<br />see errors like 'Unable to get the application list, check your<br />connection to the server...'. When my session to the console timed-out,<br />the operation I was trying to do (renaming of app) didn't do anything,<br />but I had no clue why.<br /></span> <span class="diffaddedchars" style="background-color:#ddfade;">{quote}</span> <span class="diffcontext"><br /></span> <span class="diffremovedchars" style="background-color: #ffe7e7; text-decoration:line-through;">*</span> <span class="diffaddedchars" style="background-color:#ddfade;"><br />created AGPUSH-720<br /><br />{quote}<br /></span> <span class="diffcontext"> AFAIK it's considered a bad practice to use jQuery inside controllers.<br />You use only two $ functions: $.extend and $.ajax, which can be replaced<br />with angular.extend/angular.copy and the $resource service. If you can't<br />find a way how to do something without jQuery inside controller, maybe<br />creating of custom directive is the right solution, but this doesn't<br />seem to be the case.<br /></span> <span class="diffaddedchars" style="background-color:#ddfade;">{quote}</span> <span class="diffcontext"><br /></span> <span class="diffremovedchars" style="background-color: #ffe7e7; text-decoration:line-through;">*</span> <span class="diffaddedchars" style="background-color:#ddfade;"><br />created AGPUSH-721<br /><br />{quote}<br /></span> <span class="diffcontext"> It seems that you're using several switch statements just to assign a<br />value based on input. This seems to be a good candidate for using a map.<br /></span> <span class="diffaddedchars" style="background-color:#ddfade;">{quote}</span> <span class="diffcontext"><br /></span> <span class="diffaddedchars" style="background-color:#ddfade;"><br />I don't consider this bad practice since it explains well that some options has same value as given key:<br />https://github.com/aerogear/aerogear-unifiedpush-server/blob/f2c5a5d7d834cc74e4a66bcb966437d2de68981d/admin-ui/app/scripts/controllers/DetailController.js#L186<br /><br /><br /></span> <span class="diffcontext">* In installation.html I see stuff like <col width="45%">. I think this<br />should be rewritten to use the bootstrap grid layout (classes like<br />col-md-12...).<br /><br />* We use our custom directive lo-autofocus for the 1st input in modals.<br />Right after the modal pops-up, user has the focus on the first input and<br />pressing enter submits the modal.<br /><br />* The last one is not a problem at all, I just don't feel right about<br />it :) You use $window.location.href instead $location.absUrl(). They<br />seem to do the same, but since you don't use any other $window stuff,<br />using $location makes more sense to me.<br /><br />----<br /><br />Addressed by other issues:<br /><br />* AGPUSH-659: Notifications - they are inside ng-view pages. It looks fine and works<br />for your current purposes. But if you make some new page in the future,<br />which redirects you (i.e. creating a new entity, after submit, user is<br />redirected to entity list with newly created stuff), you may loose the<br />message. Another disadvantage of this is non-consistent L&F for feedback<br />messages. The advantage is, that you're not covering any content with<br />the message as LO does.</span>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand " style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff">
<table id="actions-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 1px">
<tr>
<td id="actions-pattern-container" valign="middle" style="padding: 0px; border-collapse: collapse; padding: 10px 0 10px 24px; vertical-align: middle; padding-left: 0">
<table align="left" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td class="actions-pattern-action-icon-container" style="padding: 0px; border-collapse: collapse; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 0px; vertical-align: middle"> <a href="https://issues.jboss.org/browse/AGPUSH-695#add-comment" target="_blank" title="Add Comment" style="color: #3b73af; text-decoration: none"> <img class="actions-pattern-action-icon-image" src="cid:jira-generated-image-static-comment-icon-cd8edbbe-dfd4-4fef-b866-84ee923ba2cd" alt="Add Comment" title="Add Comment" height="16" width="16" border="0" style="vertical-align: middle" /> </a>
</td>
<td class="actions-pattern-action-text-container" style="padding: 0px; border-collapse: collapse; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 4px; padding-left: 5px"> <a href="https://issues.jboss.org/browse/AGPUSH-695#add-comment" target="_blank" title="Add Comment" style="color: #3b73af; text-decoration: none">Add Comment</a>
</td>
</tr>
</table>
</td>
</tr>
</table>
</td>
</tr>
<!-- there needs to be content in the cell for it to render in some clients -->
<tr>
<td class="email-content-rounded-bottom mobile-expand" style="padding: 0px; border-collapse: collapse; color: #fff; padding: 0 15px 0 16px; height: 5px; line-height: 5px; background-color: #fff; border-top: 0; border-left: 1px solid #ccc; border-bottom: 1px solid #ccc; border-right: 1px solid #ccc; border-bottom-right-radius: 5px; border-bottom-left-radius: 5px; mso-line-height-rule: exactly">
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td id="footer-pattern" style="padding: 0px; border-collapse: collapse; padding: 12px 20px">
<table id="footer-pattern-container" cellspacing="0" cellpadding="0" border="0" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="footer-pattern-text" class="mobile-resize-text" width="100%" style="padding: 0px; border-collapse: collapse; color: #999; font-size: 12px; line-height: 18px; font-family: Arial, sans-serif; mso-line-height-rule: exactly; mso-text-raise: 2px">
This message was sent by Atlassian JIRA <span id="footer-build-information">(v6.2.6#6264-<span title="ee7642271310c09537d01e5848a003c4498a0eed" data-commit-id="ee7642271310c09537d01e5848a003c4498a0eed}">sha1:ee76422</span>)</span>
</td>
<td id="footer-pattern-logo-desktop-container" valign="top" style="padding: 0px; border-collapse: collapse; padding-left: 20px; vertical-align: top">
<table style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="footer-pattern-logo-desktop-padding" style="padding: 0px; border-collapse: collapse; padding-top: 3px"> <img id="footer-pattern-logo-desktop" src="cid:jira-generated-image-static-footer-desktop-logo-2cad37f7-ac90-4970-acaf-a25b0500644f" alt="Atlassian logo" title="Atlassian logo" width="169" height="36" class="image_fix" />
</td>
</tr>
</table>
</td>
</tr>
</table>
</td>
</tr>
</table>
</body>
</html>