INN / umbrella-citylimits

CityLimits.org Site
https://citylimits.org
GNU General Public License v2.0
0 stars 2 forks source link

Client qa updates #136

Closed joshdarby closed 4 years ago

joshdarby commented 4 years ago

Changes

This pull request makes the following changes:

Why

For #133

Testing/Questions

Features that this PR affects:

Questions that need to be answered before merging:

Steps to test this PR:

Homepage top featured story:

  1. View the mobile homepage and verify the top story title has the same font size and secondary featured story titles

Special projects series landing page footer widgets

  1. Create/modify a landing page that has a name different from the selected series
  2. Make sure it has the "Widget" footer option selected
  3. Add widgets to that widget area and verify they appear on the frontend

Allow selection of empty secondary nav on landing pages

  1. Create/modify a landing page and select a secondary nav menu, then save
  2. Set the secondary nav menu back to - Select - option and save
  3. Verify no secondary menu appears on the frontend

Remove hardcoded newsletter signup from new series template

  1. View a landing page using the special projects series template and verify no newsletter signups are present
  2. Add the newsletter signup widget to the above landing page and verify it works and looks ok

Allow selection of different menu for open toggled mobile nav

  1. Go to menus in the admin panel
  2. Add a menu to the Main Navigation menu location
  3. Add a menu to the Mobile Sticky Main Nav menu location
  4. Verify the main desktop nav is correct and the expanded mobile nav is correct and match their selected menus on their respective menu locations
benlk commented 4 years ago

Instead of hiding the mobile popup on all pages, is there any reason we shouldn't remove it outright?

https://github.com/INN/umbrella-citylimits/blob/775d526602313da246e5d24b39a1e9d0966e5de7/wp-content/themes/citylimits/functions.php#L337-L340

joshdarby commented 4 years ago

Instead of hiding the mobile popup on all pages, is there any reason we shouldn't remove it outright?

@benlk I'm probably wrong here, but wouldn't that remove it from desktop too? I know they only want it removed from mobile currently.

benlk commented 4 years ago

With CSS, we're hiding .footer.mobile, and the only HTML/PHP I can find that has that combination of classes is /partials/newsletter-signup-popover.php, where the element with .footer.mobile contains everything within that partial. The other newsletter-signup- partials have other combinations of classes.

Everything else is :shipit:

joshdarby commented 4 years ago

With CSS, we're hiding .footer.mobile, and the only HTML/PHP I can find that has that combination of classes is /partials/newsletter-signup-popover.php, where the element with .footer.mobile contains everything within that partial. The other newsletter-signup- partials have other combinations of classes.

Alright, you've convinced me!

joshdarby commented 4 years ago

@benlk Removed in https://github.com/INN/umbrella-citylimits/pull/136/commits/cb646ab649d18a0c9938756944be0847fe65c2c2