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

Disable 'Upload' button when no file is chosen. #11983

Merged
merged 4 commits into from Oct 21, 2016
Merged

Disable 'Upload' button when no file is chosen. #11983

merged 4 commits into from Oct 21, 2016

Conversation

pkomanek
Copy link
Contributor

Fixing the bug bellow by disabling the 'Upload' button in automate import/export page when no file is chosen.

Screenshots

before:

before

after:

after

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1383174

@pkomanek
Copy link
Contributor Author

@miq-bot add_labels automate, ui, bug

@gmcculloug
Copy link
Member

@eclarizio Please review

$('#upload-datastore-import').prop('disabled', true);
}
}
);
Copy link
Member

Choose a reason for hiding this comment

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

I think this change is fine, but we should probably move it so that we don't have a bunch of logic sitting in the view. I'd propose to move this whole block into app/assets/javascripts/import.js, and into the ImportSetup object, and then just call something like ImportSetup.setUpUploadDatastoreImportButton() or something with a better name than I can come up with right now 😛 .

This way, we can also add specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eclarizio I moved the JS like you said. I also let your name of the function. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the function name to setUpUploadImportButton, because of making rest of the upload buttons to use this function.

@eclarizio
Copy link
Member

Cool, looks good now, but can we add a spec for the code in the import.js file? There is already a spec/javascripts/import_spec.js, so you'd just have to add a bit to that. Let me know if you have any questions about setting up a js spec.

@pkomanek pkomanek changed the title Disable 'Upload' button when no file is chosen. [WIP] Disable 'Upload' button when no file is chosen. Oct 19, 2016
@pkomanek
Copy link
Contributor Author

@miq-bot add_label wip

@miq-bot miq-bot added the wip label Oct 19, 2016
@miq-bot
Copy link
Member

miq-bot commented Oct 21, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@pkomanek
Copy link
Contributor Author

@miq-bot remove_label wip

@pkomanek pkomanek changed the title [WIP] Disable 'Upload' button when no file is chosen. Disable 'Upload' button when no file is chosen. Oct 21, 2016
@miq-bot miq-bot removed the wip label Oct 21, 2016
@miq-bot
Copy link
Member

miq-bot commented Oct 21, 2016

Checked commits pkomanek/manageiq@a3ebcdd~...1858835 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. 🏆

@eclarizio
Copy link
Member

LGTM 👍

@gmcculloug gmcculloug merged commit 4ae1916 into ManageIQ:master Oct 21, 2016
@gmcculloug gmcculloug added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 21, 2016
chessbyte pushed a commit that referenced this pull request Oct 22, 2016
…te_import-export_page

Disable 'Upload' button when no file is chosen.
(cherry picked from commit 4ae1916)

https://bugzilla.redhat.com/show_bug.cgi?id=1383174
@chessbyte
Copy link
Member

Euwe Backport details:

$ git log -1
commit b0efb4921f11d30df48d0f1b731fe5ecac09f7a2
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Fri Oct 21 15:46:20 2016 -0400

    Merge pull request #11983 from pkomanek/spinner_rotate_fix_for_automate_import-export_page

    Disable 'Upload' button when no file is chosen.
    (cherry picked from commit 4ae1916e478646187420ec06d1feb84a97c56cc6)

    https://bugzilla.redhat.com/show_bug.cgi?id=1383174

@chessbyte
Copy link
Member

@pkomanek pkomanek deleted the spinner_rotate_fix_for_automate_import-export_page branch October 27, 2016 11:31
@simaishi
Copy link
Contributor

simaishi commented Nov 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants