Skip to content

More permissive check for showing location alerts#353

Merged
matt-bernhardt merged 1 commit intomainfrom
uxws-1368-location-alerts
May 4, 2022
Merged

More permissive check for showing location alerts#353
matt-bernhardt merged 1 commit intomainfrom
uxws-1368-location-alerts

Conversation

@matt-bernhardt
Copy link
Copy Markdown
Member

@matt-bernhardt matt-bernhardt commented May 3, 2022

Why are these changes being introduced:

  • The most recent location template - introduced last fall in Adds Location 2021 page template #344 - uses too
    strict a comparison to determine whether a location alert will be
    shown - the === operator used is different from that used by the older
    location template (which uses ==). I didn't realize initially the
    using the stricter comparison would never result in the alert being
    shown, it seems.

Compare the new location template at https://github.com/MITLibraries/MITlibraries-parent/blob/main/content-location-2021.php#L74
with the older template at https://github.com/MITLibraries/MITlibraries-parent/blob/main/content-location.php#L97

Relevant ticket(s):

How does this address that need:

  • This change reverts the new location template to using the more
    permissive == operator instead of ===.

Document any side effects to this change:

  • I'm still not entirely sure about how the logic of this check works,
    so there may be some aspect - but I doubt it. This reverts to the same
    comparison that is working for the other location template.
  • As expected, CodeClimate is flagging the use of the == operator - which is why I originally changed this check in the first place. That's going to have to be accepted for now.

How can a reviewer manually see the effects of these changes?

This branch is current present on the staging server. If you look at the Hayden Library page, you'll note a location alert being shown about the elevators being out of service.

Screenshots (if appropriate)

Staging (showing location alert)
Screen Shot 2022-05-03 at 3 55 48 PM

Production (without alert)
Screen Shot 2022-05-03 at 3 56 11 PM

Todo:

  • Documentation
  • Stakeholder approval

Requires new or updated plugins, themes, or libraries?

NO

Requires change to deploy process?

NO

** Why are these changes being introduced:

* The most recent location template - introduced last fall - uses too
  strict a comparison to determine whether a location alert will be
  shown - the === operator used is different from that used by the older
  location template (which uses ==). I didn't realize initially the
  using the stricter comparison would never result in the alert being
  shown, it seems.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/uxws-1368

** How does this address that need:

* This change reverts the new location template to using the more
  permissive == operator instead of ===.

** Document any side effects to this change:

* I'm still not entirely sure about how the logic of this check works,
  so there may be some aspect - but I doubt it. This reverts to the same
  comparison that is working for the other location template.
@matt-bernhardt matt-bernhardt requested review from JPrevost and jazairi May 3, 2022 20:01
@matt-bernhardt matt-bernhardt merged commit 97cba83 into main May 4, 2022
@matt-bernhardt matt-bernhardt deleted the uxws-1368-location-alerts branch May 4, 2022 13:52
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.

2 participants