Merged
Conversation
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
| $val = $arMain[ array_rand( $arMain ) ]; | ||
| ?> | ||
| <?php if ( $val != '' ) : ?> | ||
| <img src="<?php echo $val; ?>" data-thumb="<?php echo $val; ?>" alt="<?php the_title(); ?>" /> |
There was a problem hiding this comment.
This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.
| $val = $arMain[ array_rand( $arMain ) ]; | ||
| ?> | ||
| <?php if ( '' != $val ) : ?> | ||
| <img src="<?php echo $val; ?>" data-thumb="<?php echo $val; ?>" alt="<?php the_title(); ?>" /> |
There was a problem hiding this comment.
This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.
matt-bernhardt
force-pushed
the
uxws-1205-location-2021-template
branch
from
74965c1 to
8237022
Compare
| $val = $arMain[ array_rand( $arMain ) ]; | ||
| ?> | ||
| <?php if ( '' != $val ) : ?> | ||
| <img src="<?php echo $val; ?>" data-thumb="<?php echo $val; ?>" alt="<?php the_title(); ?>" /> |
There was a problem hiding this comment.
This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.
| $val = $arMain[ array_rand( $arMain ) ]; | ||
| ?> | ||
| <?php if ( '' !== $val ) : ?> | ||
| <img src="<?php echo $val; ?>" data-thumb="<?php echo $val; ?>" alt="<?php the_title(); ?>" /> |
There was a problem hiding this comment.
This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.
Member
Author
There was a problem hiding this comment.
Never change, accesslint. There is, in fact, an alt attribute on this image. It is the third and final attribute in the line on which you are commenting.
matt-bernhardt
force-pushed
the
uxws-1205-location-2021-template
branch
from
37aabd4 to
921c484
Compare
** Why are these changes being introduced: * With the re-opening of the Hayden Library after its renovation, there is a desire to showcase content on its page that is not supported by our current location template. However, the changes coming for this page would not be appropriate for other current location pages. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/UXWS-1205 ** How does this address that need: * This adds a second template option for location pages. This will allow pages to be converted from the old to new format individually, as their content is updated for the new layout. * Specifically, this adds page-location-2021.php, which is identified as "Location (2021) Template" in the WordPress page editor. This page template then loads content-location-2021.php, following the pattern used elsewhere in the theme. * Some styles specific to this new template are also added to the _locations SCSS partial. * The file comments for the legacy location content has been updated. ** Document any side effects to this change: * In order for a successful deployment, two configuration changes will be necessary within the Advanced Custom Fields plugin. Specifically: 1. The "Location" field group will need to be edited to show on this new page template, not just the legacy template. 2. The "Location Attributes" field group will need a new field added, named "content_top_2021". This will be a WYSIWYG field. * CodeClimate is currently flagging ~80 issues in the new template, which are holdovers from the legacy template. Most are stylistic concerns, but the lack of content escaping may need discussion. There is a ticket in the UX backlog to coordinate this work later: UXWS-1206. * The stylesheet changes in this PR include some copy-paste of styles defined elsewhere, with new selector rules. An example is the .tabcontent class, which is defined for small screen widths in _tabs. This new template will need those styles for all screen widths. * The theme version is bumped to 1.12.0.
matt-bernhardt
force-pushed
the
uxws-1205-location-2021-template
branch
from
921c484 to
a16e192
Compare
jazairi
approved these changes
Aug 17, 2021
Contributor
jazairi
left a comment
jazairi
left a comment
There was a problem hiding this comment.
I think this all makes sense to me. Thanks for chatting about it in Slack. Documenting some conclusions here:
- I'm comfortable with setting aside the codeclimate issues about escaping rendered content, given that it's a fairly complex issue that's likely out of scope of this work, and it's documented as a ticket in the UXWS Jira.
- I'm also comfortable ignoring the remaining codeclimate issues, since they all seem to be style complaints that don't worry me.
- I don't think additional documentation is necessary re: the dependent config in Advanced Custom Fields, since we shouldn't need to reconfigure it after the first time.
Nice work on this! 
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status
Use labels (
review needed,in progress, orpaused) to declare statusWhat does this PR do?
This introduced a new page template to the theme, "Location (2021)". This is a new option for formatting a library location page, with these differences:
Helpful background context (if appropriate)
This is discussed in greater depth in the linked Jira ticket.
How can a reviewer manually see the effects of these changes?
You can see the new template in action on the staging site, for the Hayden page.
What are the relevant tickets?
Todo:
Requires new or updated plugins, themes, or libraries?
NO
Requires change to deploy process?
No, HOWEVER:
This change should be deployed in conjunction with configuration changes to the Advanced Custom Fields plugin. These are detailed in the commit message, but I'm open to adding them somewhere more sustainable. I'd love to hear options for this.