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

tests : Add tests for CoreList block. #22

Merged
merged 15 commits into from
Sep 26, 2024
Merged

tests : Add tests for CoreList block. #22

merged 15 commits into from
Sep 26, 2024

Conversation

Ta5r
Copy link
Collaborator

@Ta5r Ta5r commented Sep 20, 2024

Tracking wpengine#298


What:

This PR backfills tests for the Core List and Core List Item block and their attributes.

Tested attributes:

CoreListAttributes

  • anchor
  • backgroundColor
  • className
  • cssClassName
  • fontFamily
  • fontSize
  • gradient
  • lock
  • ordered
  • placeholder
  • reversed
  • start
  • style
  • textColor
  • type
  • values

CoreListItemAttributes

  • className
  • content
  • fontFamily
  • fontSize
  • lock
  • style

Untested fields:

  • CoreListAttributes.metadata - @todo

Exposed issues:

  • CoreListAttributes.cssClassNames - This is not working in the tests.

These issues are noted in comments within the test file and should be addressed in future updates.

@Ta5r Ta5r self-assigned this Sep 20, 2024
@justlevine justlevine changed the title tests : Add tests for Core List block. tests : Add tests for CoreList block. Sep 21, 2024
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

Looks great!

CoreListItem.parentClientId - This is is null in the response.

It's null when editorBlocks arent flat. To cover this, add a new test with a minimal $query (of just the clientId + parent Client Ids on the list + listItem and compare.

Once that and the minimal comments are handled (and we take a stab at the b/c compatibility issue on WP 6.1) this should be good to merge.

To handle WP 6.1 failing, see the [slack instructions[(https://rtcamp.slack.com/archives/CK2CEH4F6/p1727126999500269?thread_ts=1726011552.525109&cid=CK2CEH4F6)

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

@tanay in 3972c4a I simplified things, and ensured we're only gating the b/c stuff instead of all the list item props.

Go ahead and open a PR upstream 🚀

@justlevine justlevine marked this pull request as ready for review September 24, 2024 11:57
@justlevine justlevine added the has-upstream-pr A PR has been opened against wpengine's repo label Sep 24, 2024
@justlevine justlevine merged commit 1af6568 into main Sep 26, 2024
10 checks passed
@justlevine justlevine deleted the core-list-test branch October 18, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-upstream-pr A PR has been opened against wpengine's repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants