Topology header - fix Enter in search #1005
Conversation
…n* buttons to prevent clicks on toolbar buttons within a form from submitting that form
so that pressing enter in the search field doesn't submit the form
For some reason, pressing enter in an input triggers a *click* event on the immediately following button. (Maybe something to do with a form without any input type=submit?) Thus, and given the restrictions on this particular template, working around the issue by providing an invisible button which does the same thing as form submit does. (Could have been just a `%button.hidden` by itself, except all `%button` elements in toolbars have a miqToolbarOnClick as the click handler.) The change in `miqHideSearchClearButton` is to find the right button even if it is not an immediate sibling.
|
Checked commits https://github.com/himdel/manageiq-ui-classic/compare/c9f4dca69bcee74b462201aae339e3ebbc766dac~...e6059ecd37a9cc154f66c0ef0927e853b4190e2a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Yes, I'm completely aware that the fix that I end up making for the above BZ would be 100% non-backportable for Fine. EDIT: Looks like you have also addressed the BZ. |
| %button.hidden{'data-function' => 'miqCallAngular', | ||
| 'data-function-data' => {:name => 'searchNode', | ||
| :args => []}.to_json} | ||
|
|
There was a problem hiding this comment.
Works great... I do not see the annoying page Refresh like I did before and it also does the search when I hit enter.
Although what I'm not too happy about is that this seems like a "brute force" approach, and hence not the most logical fix.
But I'm sure you must have given a fair amount of thought to this, and this is probably as good as it can get.
I understand that this is kinda tricky to fix.
There was a problem hiding this comment.
Hey, I completely agree this is a bad approach, I just couldn't fine any better yet :(.
Ultimately this should never be doing the miqCallAngular magic, except right now, custom toolbars is the one place where we can't use angular.. but I'll try to keep this in mind (well, in the issue anyway :D) and convert this as soon as we can :).
|
Tested in UI. LGTM. |
|
@himdel: can you, please, set the labels on this PR? (trying to guess some) |
In #955, we merged the various toolbar-like things into a proper custom toolbar. But this introduced a regression - since the whole custom toolbar is a form, pressing enter in the search box would submit the form. (ref: #955 (comment))
So.. catching the submit event to trigger the same action as the same button.
..except for some reason, when pressing Enter in an input in this form, the browser creates a click event on the button immediately following that input - which is the clear button, so .. not ideal :).
Adding a hidden button that triggers the right action as a workaround, and updating the
miqHideSearchClearButtoncode to be able to cope.The long term goal for this is to have this as an angular component that talks with the right TopologyController via an observable, but turns out we can't have angular in custom toolbars quite yet - ManageIQ/ui-components#64.
Cc @AparnaKarve, @skateman
Aparna.. this should fix the issue, but not sure how much luck you'll have using this for https://bugzilla.redhat.com/show_bug.cgi?id=1401823 in
fine(assuming that BZ is meant forfine), since before #955, most topologies usedshared/_topology_header.html.haml, which is not a proper toolbar but is angular.