Skip to content
Snippets Groups Projects

Resolve #3461101 "Remove sdc test dependency"

2 unresolved threads

Closes #3461101

Merge request reports

Approval is optional
Code Quality is loading
Test summary results are being parsed

Merged by Wim LeersWim Leers 6 months ago (Jul 16, 2024 8:52am UTC)

Merge details

  • Changes merged into with 272c596a.
  • Did not delete the source branch.

Pipeline #225364 passed

Pipeline passed for 272c596a on 0.x

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
79 79 if (propData.expression) {
80 80 preparedModel[selectedComponent as keyof PreparedModel][propName].expression = propData.expression;
81 81 }
82 // The current value of the prop.
83 if (selectedModel[propName]) {
84 preparedModel[selectedComponent as keyof PreparedModel][propName].value = selectedModel[propName];
85 }
82
83 // The current value of the prop, or an empty string so the `value` is at
84 // least present.
85 preparedModel[selectedComponent as keyof PreparedModel][propName].value = selectedModel[propName] || '';
  • I had to make this change in order for the props edit form to work. It probably isn't technically in scope of this issue, but something this issue made it much easier to encounter this normal/valid use case.

    Perhaps all manual testing prior to this was with SDCs that had default values available for their props, but while reviewing this MR, it demonstrated that each prop must have a value key, even if there's currently no value. If this theory is correct, I'd like to just sneak this change in with the MR since it nicely reproduces the issue then fixes it. If I'm incorrect and this is actually a problem with the MR itself then of course lets address the underlying problem.

  • Author Maintainer

    Yes, see \Drupal\experience_builder\PropSource\StaticPropSource::parse() and \Drupal\experience_builder\PropSource\StaticPropSource::__toString(): each static prop source must have an expression and a value. :thumbsup:

  • Please register or sign in to reply
  • Ben Mullins added 1 commit

    added 1 commit

    • d2730a43 - logs that shouldn't be there

    Compare with previous version

  • 1 <div style="font-family: Helvetica, Arial, sans-serif; width: 100%; height: 100vh; background-color: #f5f5f5; display: flex; justify-content: center; align-items: center; flex-direction: column; text-align: center; padding: 20px; box-sizing: border-box;">
    1 <div {{ attributes }} style="font-family: Helvetica, Arial, sans-serif; width: 100%; height: 100vh; background-color: #f5f5f5; display: flex; justify-content: center; align-items: center; flex-direction: column; text-align: center; padding: 20px; box-sizing: border-box;">
    2 2 <h1 style="font-size: 3em; margin: 0.5em 0; color: #333;">{{ heading }}</h1>
    3 3 <p style="font-size: 1.5em; margin: 0.5em 0; color: #666;">{{ subheading }}</p>
    4 4 <div style="margin-top: 1em;">
    5 <button style="background-color: #007BFF; color: white; border: none; padding: 15px 30px; font-size: 1em; margin: 0 10px; cursor: pointer; border-radius: 5px;">
    5 <button formaction="{{ cta1href }}" style="background-color: #007BFF; color: white; border: none; padding: 15px 30px; font-size: 1em; margin: 0 10px; cursor: pointer; border-radius: 5px;">
    • I know this is a dummy component, but this is a link, let's use an a with a href

    • Author Maintainer

      This entire component is pretty weird, with buttons that don't do anything. I'd rather keep evolving this component out of scope here.

      The only reason I added the cta1href prop to my-hero is to not lose/reduce test coverage.

      Making my-hero make sense is out-of-scope. Making XB easier to contribute to by not requiring the sdc_test module anymore is more urgent than improving a clearly intentionally silly dummy component :blush:

    • Please register or sign in to reply
  • merged

  • Please register or sign in to reply
    Loading