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

Removes design tokens, scripts, references, updates theme.json, etc. #444

Draft
wants to merge 17 commits into
base: 5.x-RC
Choose a base branch
from

Conversation

cssgirl
Copy link
Member

@cssgirl cssgirl commented Apr 23, 2024

Testing AC:

  • Check that correct packages are installed and build/watch commands work as expected
  • Make sure all references to gesso functions and variables have been removed and replaced with the equivalent CSS variable generated from theme.json
  • Make sure updated patterns, templates work as expected
  • Review theme.json updates & additions
  • Check that the site editor options we want are available and the ones we are not ready to use yet are disabled/hidden
  • Check rename command is working

@coreylafferty coreylafferty changed the base branch from 5.x to 5.x-RC April 26, 2024 19:52
@jackmakesthings
Copy link
Contributor

@cssgirl Something I'm noticing - npm run rename doesn't seem to work. I don't think this was caused by your changes - it's not working in the base branch either. Seems like the dependencies aren't getting installed. Can you look into that?

Maybe we just need to add the dependencies to package.json manually?


const inquirer = require('inquirer');
const path = require('path');
const fsPromises = require('fs/promises');

@tropicandid
Copy link
Contributor

@cssgirl Thank you! Would you mind dropping some testing AC in here so we can stress test the changes? Just any editorial expectations that you think might have been effected that we can verify.

@tropicandid tropicandid self-requested a review May 8, 2024 19:24
@cssgirl
Copy link
Member Author

cssgirl commented May 10, 2024

@tropicandid @jackmakesthings - I've added some preliminary AC above!

I am updating the readme and have one small batch of changes (color variable names) - otherwise this should be ready to pull down and test!

@jackmakesthings
Copy link
Contributor

Will we update the gesso version number once this is merged?

@cssgirl
Copy link
Member Author

cssgirl commented May 15, 2024

Last item I am working on - fixing the theme's rename functionality.

@cssgirl cssgirl requested a review from liquilife May 15, 2024 17:40
@cssgirl
Copy link
Member Author

cssgirl commented May 16, 2024

@cssgirl Something I'm noticing - npm run rename doesn't seem to work.

@jackmakesthings
Fixed! Looks like it never actually worked. Seems like it was a copy of the same rename functionality we have in the block library and some variable names & stuff and the inquirer package were overlooked. Should be working now!

@cssgirl
Copy link
Member Author

cssgirl commented May 16, 2024

One last set of changes to be made:

  1. remove all CSS for blocks provided by the f1 block library (except for cards) and move those styles into the appropriate block folders in the f1 block library.
  2. Update references to images in kitchen sink pattern (and other patterns as needed) so no broken images.

@tropicandid
Copy link
Contributor

tropicandid commented May 20, 2024

@cssgirl I am seeing two small things OOTB. I've attached screenshots.

  1. In the editor, now that we've removed the colors, when I select/highlight content, the text goes white and is illegible.
Screenshot 2024-05-20 at 3 01 24 PM
  1. The pattern for the Article displays the block recovery message by default.
Screenshot 2024-05-20 at 3 04 54 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants