Skip to content

DO NOT MERGE - CI and Pantheon updates for parent theme#357

Open
matt-bernhardt wants to merge 19 commits intoMITLibraries:mainfrom
matt-bernhardt:main
Open

DO NOT MERGE - CI and Pantheon updates for parent theme#357
matt-bernhardt wants to merge 19 commits intoMITLibraries:mainfrom
matt-bernhardt:main

Conversation

@matt-bernhardt
Copy link
Copy Markdown
Member

@matt-bernhardt matt-bernhardt commented Jun 1, 2022

During the creation of the prototype WordPress site on Pantheon in January 2022, I forked our parent and child themes into projects I could manipulate more easily. Now that we are anticipating putting this work into production, I wanted to see how ugly the merge would be to capitalize on that effort in the current codebase - or if it would be necessary to just carry forward with the forks as new themes.

This PR is how I'm checking on that question.

Status

Use labels (review needed, in progress, or paused) to declare status

What does this PR do?

A few sentences describing the overall goals of the pull request's commits.
Why are we making these changes? Is there more work to be done to fully
achieve these goals?

Helpful background context (if appropriate)

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

What are the relevant tickets?

Screenshots (if appropriate)

Todo:

  • Documentation
  • Stakeholder approval

Requires new or updated plugins, themes, or libraries?

YES | NO

Requires change to deploy process?

YES | NO

matt-bernhardt and others added 19 commits February 17, 2022 11:28
This applies to functions.php and lib/news.php
This is applied to:

- hours location images
- lightbox library
- guide loading on the home page
- map icons within mapJS
- loading fonts.js for productionJS and homeJS

This also removes an apaprently-unused file - js/make.googlemap.js
It wasn't possible to use the i18n approach here, so instead
we are just using relative paths back to the images and libs
directories.
- Update theme name and screenshot
- Adds sidebar styles found in Customizer
Finishes fork groundwork, adds Composer support
Following the GUI provided by GitHub for enabling Actions...
Establish build process using GitHub Actions
Update build workflows based on lessons learned in other projects
** Why are these changes being introduced:

* We need to be able to load system alerts from external sources, now
  that we are going to be deploying many WordPress applications. The
  status quo is to load from a hard-coded URL on the same domain (from
  the parent site). That needs to be more flexible.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/engx-147

** How does this address that need:

* This defines a theme setting for the alert URL, which can be edited by
  site builders. That value is made available to site javascript using
  the ALERT_URL constant, rendered by wp_add_inline_script().
* It also defines a dashboard settings form for modifying that setting
  (which is in a separate class, although I'm not sure that is the best
  choice).

** Document any side effects to this change:

* There are now more files in the lib/ folder, which may not be ideal.
  The preferrable theme organization is unclear to me, so I've gone
  with a folder that already exists.
Add support for loading alerts from remote systems
Copy link
Copy Markdown

@accesslint accesslint Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are accessibility issues in these changes.

'p' => array(),
);

$template = '<input type="text" name="%s" value="%s" id="%s" size="60"><p>This should be the fully-qualified URL for the WordPress application which hosts Alert content. The original value was https://libraries.mit.edu/wp-json/wp/v2/posts/</p>';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

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.

1 participant