baltimorecounty / BCPL-assets

Client side assets for the Baltimore County Public Library website
4 stars 0 forks source link

Polaris "More Search Options" link broken #637

Open danfox01 opened 5 years ago

danfox01 commented 5 years ago

On the Advanced search page (https://catalog.bcpl.lib.md.us/polaris/search/default.aspx?ctx=1.1033.0.0.5&type=Advanced) the "More Search Options" link doesn't do anything.

I'm not sure what the expected behavior should be -- it's probably been broken for a quite some time, but I believe it should fire a modal with more options.

Can a dev please take a look and see if anything is blocking that functionality?

tmccoy529 commented 5 years ago

@danfox01 it appears that a JavaScript function isn't loading on the page which is used to open this modal. Could be that it doesn't exist since I cant find any references to it in the code other than being added to the element. I'll let you know when I have a fix in place.

tmccoy529 commented 5 years ago

@danfox01 I believe I have a solution in place. However this breaks the Notary Service as I've moved over a new branch without that fix in place. If you could verify that this is now working I will do a new PR for this issue and then we can hopefully move both branches into integration fixing both these bugs.

danfox01 commented 5 years ago

@tmccoy529 this looks good. You're good to go. Thanks for the quick work.

danfox01 commented 5 years ago

@tmccoy529, please shoot me an update on Monday (1/28) -- I'd like to get this and #636 merged and into prod early in the week. Thanks!

tmccoy529 commented 5 years ago

@martypowell when do you think we can get this pushed over for Dan?

danfox01 commented 5 years ago

@jdomasky, I'd like to run the general Polaris test plan on this. Can you please tackle this sometime in the next few days?

jdomasky commented 5 years ago

No problem. I'll let everyone know before I start.

danfox01 commented 5 years ago

@jdomasky, do you have an ETA on this?

jdomasky commented 5 years ago

Yes, I plan to test today, by early afternoon. If the test is successful, I'll post the results here. Otherwise, I'll follow up with @danfox01 directly.

jdomasky commented 5 years ago

@tmccoy529 @danfox01 @martypowell I just finished the General Polaris Test Plan. Polaris dev passed!

martypowell commented 5 years ago

@danfox01 @tmccoy529 this has been approved, we could release this a hotfix?

danfox01 commented 5 years ago

@martypowell I'm good with that. I'm also going to schedule weekly release meetings like we used to have -- I think the recurring appointment expired. I'd like to try to start planning set releases now that we're getting more stuff knocked out.

danfox01 commented 5 years ago

@tmccoy529 , please work with Mike Kollin to release to prod when you're ready.

jdomasky commented 5 years ago

@tmccoy529 @danfox01 @martypowell This morning, in Polaris prod and dev, "More Search Options" works on desktop, but the feature seems to have no effect on mobile. As we know, in a desktop browser, after you click "Set Search Options" in the "More Search Options" modal, a yellow DIV appears on the Advanced Search form. On Android Chrome and iPhone Safari, after I clicked "Set Search Options," the yellow DIV didn't appear on my own device or in BrowserStack. When I proceeded with an Advanced Search anyway, the limiters I selected in "More Search Options" were ignored. I'm guessing this problem isn't an element display issue because the yellow box displays in a desktop browser when the browser window is extremely narrow. Here's a screen capture of the BrowserStack console log while using Android Chrome:

20190206_browserstack_console

martypowell commented 5 years ago

one solution to this would be to warp the failing line like this:

if (toastr) {
  // the vendor's toastr code
}
danfox01 commented 5 years ago

Moving this back to in progress.

martypowell commented 5 years ago

@tmccoy529 @sgrg1 any updates on this?

martypowell commented 5 years ago

We've tried this on 3 physical (1 iphone, 2 androids) devices and cannot recreate. @danfox01 @jdomasky have you tried on your physical phones?

danfox01 commented 5 years ago

@jdomasky can you please verify if this issue still exists? If so, please document the conditions that lead to it for Tim, and if not, we can close.

jdomasky commented 5 years ago

@tmccoy529 @danfox01 The issue with the appearance of the yellow DIV on mobile is no longer occurring. Yay! However, there’s a new, possibly related problem on both desktop and mobile: The More Search Options modal is clearing the Advanced Search form when the modal is submitted.

Steps to reproduce:

  1. Type a search term in the Advanced Search form.
  2. Click on More Search Options.
  3. Choose an option in the modal.
  4. Click the Set Search Options button in the modal.
  5. When the modal disappears, anything you typed in the Advanced Search Form is erased.

This doesn’t happen if you simply close the modal. I don’t think this issue was happening last month when I documented the problem.

I believe this is a problem because most users think it’s safe to open a modal and they don’t expect their search terms to be deleted.

I noticed that the whole page seems to reload when the modal is submitted. This may be the cause of the undesired form clearing.

tmccoy529 commented 5 years ago

@danfox01 @martypowell @jdomasky I would agree this was not functioning this way the last round of testing. This is too obvious to have been missed. Nothing should have changed in Polaris though as that was the last PR done. I'll start looking into it.

tmccoy529 commented 3 years ago

@jdomasky Im trying to replicate this and I cannot. When I have text entered into the search box and select any options in the modal the text is not cleared anymore in Chrome. Maybe Im missing something?

jdomasky commented 3 years ago

@tmccoy529 I just replicated the problem in #637 "Polaris 'More Search Options' link broken" in desktop Chrome. Are you following the same steps as me? Here are the steps to replicate:

  1. Browse: https://catalog.bcpl.lib.md.us/polaris/search/default.aspx?ctx=1.1033.0.0.6&type=Advanced
  2. Type a search term in the first field (next to "Find:" label, next to "Any Field" dropdown).
  3. Scroll down a little and click the "More Search Options" link.
  4. In the popup modal, choose "Arbutus Branch".
  5. Click the "Set Search Options" button in the modal.
  6. When the modal disappears, observe that the search term you previously typed has disappeared.

Based on this, it seems that my comment on March 27, 2019 is still valid. At your convenience, please let me know if you can confirm the above, or if you have other thoughts. Thanks!

FYI @hburnopp @mcorasan @theilman7 @tthbcmd