Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete proxy backend #1606

Merged
merged 13 commits into from
Sep 22, 2016
Merged

Delete proxy backend #1606

merged 13 commits into from
Sep 22, 2016

Conversation

jykae
Copy link
Contributor

@jykae jykae commented Sep 21, 2016

Closes #1509

Proposed changes

  • delete proxyBackend feature
  • improve code style where appropriate with my new friend ESLint

@jykae jykae added this to the Sprint 31 milestone Sep 21, 2016
@brylie
Copy link
Contributor

brylie commented Sep 21, 2016

The following submit/delete button group code works properly, where clicking delete does not trigger the update event:

<div id="proxy-form-buttons" class="btn-group pull-left">
  <!-- submit button -->
  <button
    type="submit"
    class="btn btn-success"
    id="save-proxy-button">
    {{_ "proxyBackendForm_saveButton" }}
  </button>
  <!-- delete button -->
  <button
    type="button"
    class="btn btn-danger"
    id="delete-proxy-button">
    Delete
  </button>
</div>

@brylie
Copy link
Contributor

brylie commented Sep 21, 2016

You will also need to change your form.less:

/* Change the following */
#save-proxy-button {
  #margin-top: 1em;
}

/* to */
#proxy-form-buttons {
  #argin-top: 1em;
}

@jykae
Copy link
Contributor Author

jykae commented Sep 21, 2016

@brylie Ok, thanks. I try out. Just pushed functionality.

@jykae
Copy link
Contributor Author

jykae commented Sep 21, 2016

@brylie Seems to work. I push also UI changes.

@jykae
Copy link
Contributor Author

jykae commented Sep 21, 2016

@apinf/developers ready for review

@brylie brylie self-assigned this Sep 22, 2016
@brylie
Copy link
Contributor

brylie commented Sep 22, 2016

This looks good. Try to reduce the amount of nesting involved in the delete event handler. That will make the code slightly easier to read.

// Check proxyBackend exists
if (proxyBackend) {
// Check if proxyBackend type is apiUmbrella & it has id
if (proxyBackend.apiUmbrella && proxyBackend.apiUmbrella.id) {
Copy link
Contributor

@brylie brylie Sep 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there is a syntactic reason to separate these statements, combine them into one if statement. Separate each conditional on its own line:

if (proxyBackend &&
 proxyBackend.apiUmbrella &&
 proxyBackend.apiUmbrella.id ) {
  // do something
}

(deleteError) => {
if (deleteError) {
const deleteErrorMessage = TAPi18n.__('proxyBackendForm_deleteErrorMessage');
sAlert.error(`${deleteErrorMessage}:\n ${deleteError}`);
Copy link
Contributor

@brylie brylie Sep 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of template strings 😄

@brylie brylie merged commit 6fed6ba into develop Sep 22, 2016
@brylie brylie deleted the feature/delete-proxy-backend branch September 22, 2016 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants