Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDSS-1268: Removed white option from paragraph section background colors #430

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

jenbreese
Copy link
Contributor

READY FOR REVIEW

Summary

  • Removed the white background option from layouts.

Review By (Date)

  • as time allows

Criticality

  • normal

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Navigate to to a new page.
  3. Create a row.
  4. Verify you can no longer add a white background to a row.

Front End Validation

  • Is the markup using the appropriate semantic tags and passes HTML validation?
  • Cross-browser testing has been performed?
  • Automated accessibility scans performed?
  • Manual accessibility tests performed?
  • Design is approved by @ user?

Backend / Functional Validation

Code

  • Are the naming conventions following our standards?
  • Does the code have sufficient inline comments?
  • Is there anything in this code that would be hidden or hard to discover through the UI?
  • Are there any code smells?
  • Are tests provided? eg (unit, behat, or codeception)

Code security

General

  • Is there anything included in this PR that is not related to the problem it is trying to solve?
  • Is the approach to the problem appropriate?

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

  • JIRA ticket(s)
  • Other PRs
  • Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
  • Anyone who should be notified? (@mention them here)

Resources

@github-actions github-actions bot added the patch label May 24, 2024
Copy link
Collaborator

@joegl joegl left a comment

Choose a reason for hiding this comment

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

We can't just remove an option from a select field. If the option is used somewhere we need to change the value to an existing one. This will require a database update hook.

@jenbreese
Copy link
Contributor Author

@joegl Should I go ahead and close this and the row background colors be part of the new work from yesterday?

@joegl
Copy link
Collaborator

joegl commented May 29, 2024

@jenbreese Nope, the bg color options are going to stay where they are. Only the width options are getting moved.

@joegl
Copy link
Collaborator

joegl commented May 29, 2024

@jenbreese I added an update hook to migrate any layout paragraph with a white background color to "none". We should be good to merge this once code freeze is up.

@joegl
Copy link
Collaborator

joegl commented May 29, 2024

@jenbreese Here are some steps to test the database update on local or gitpod:

  1. In the docroot/profiles/sdss/sdss_profile/modules/sdss_layout_paragraphs/src/Plugin/Layout/SdssLayoutParagraphBase.php file, re-add the "white" bg_color that was removed (only for testing)
  2. Create a new page with a layout paragraph using the "white" background color and save it
  3. Reset the schema version for sdss_profile with drush (to re-trigger database update): drush "\Drupal::service('update.update_hook_registry')->setInstalledVersion('sdss_profile', 10006);"
  4. Run database update: drush updatedb -y
  5. Edit the page created and confirm the layout paragraph with the "white" background color is now set to "none"

@jenbreese
Copy link
Contributor Author

Hi @joegl, I tried it and it did reset the choice to none. So I think we are good to go for after the code freeze.

@joegl
Copy link
Collaborator

joegl commented May 29, 2024

Hi @joegl, I tried it and it did reset the choice to none. So I think we are good to go for after the code freeze.

Awesome, thanks Jen! We'll get this merged after the 6/5 deploy/code freeze.

Copy link
Collaborator

@joegl joegl left a comment

Choose a reason for hiding this comment

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

LGTM (and Jen 😄 ). Thanks Jen!

@joegl joegl merged commit de48fc4 into 4.x Jun 6, 2024
5 checks passed
@joegl joegl deleted the SDSS-1268--remove-bg-white branch June 6, 2024 17:57
@joegl joegl changed the title SDSS-1268: Removed the white option for row backgrounds SDSS-1268: Removed white option from paragraph section background colors Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants