🇺🇸United States @hooroomoo

nyc
Account created on 14 September 2021, over 4 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States hooroomoo nyc

Not sure what documentation is expected or if issue summary update and comment #12 on this issue suffices. Removing tags

🇺🇸United States hooroomoo nyc

#5:

So: not using a type: string, format: uuid prop, which could then receive the host entity's UUID?

We decided to not take this approach and instead add it as a part of pageData() because

1. Can be done without needing to support UUID as a prop which isn’t supported yet.
2. Removes the need for the site builder to manually set the prop to the entity UUID.
3. Follows an existing pattern that is used to get the pageTitle and breadcrumb accessible through drupalSettings.

But because it relies on global scope, it won't actually work for other scenarios

I think the 2 scenarios you described should still be able to be done using JSON API fetch directly in the code component code. However, in the future if we find that this approach has limitations, we will revisit the UUID as prop approach.

At minimum, I think this illustrates that adding this to getPageData() is not a good idea; adding a new getEntityRouteData() or something like that would be more prudent?

I disagree, I think it makes sense to include it as part of getPageData because the entityUUID is data of the page (content entity). getPageData already provides the title of a content entity. Also I think it makes sense for a code component developer to assume that any data for a given “page”/content entity would be provided by this function. getEntityRouteData() is very Drupal-specific language and code component devs shouldn't have to know what that means.

🇺🇸United States hooroomoo nyc

hooroomoo changed the visibility of the branch 3565754-provide-entityuuid-of to hidden.

🇺🇸United States hooroomoo nyc

hooroomoo changed the visibility of the branch 3565754-provide-entityuuid-of to active.

🇺🇸United States hooroomoo nyc

hooroomoo changed the visibility of the branch 3565754-provide-entityuuid-of to hidden.

🇺🇸United States hooroomoo nyc

didn't mean to change status oops

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo created an issue.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Updated IS to add ** This should be the last issue to get merged as part of the 🌱 Plan to allow editing props and slots for exposed code components Active **

🇺🇸United States hooroomoo nyc

Few small things.

I also had a question, is there a consensus on what happens if someone adds a prop on a code component that has been published? The current behavior in the frontend is the new prop doesn't appear in the settings form but it shows up when you open the code editor which is confusing to users. So might need some UX changes for that but I don't think that is the job of this issue.

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo created an issue.

🇺🇸United States hooroomoo nyc

Un-assigning myself because I have to hand this off. Updated MR description

🇺🇸United States hooroomoo nyc

hooroomoo changed the visibility of the branch 3537654-automatically-detect-first-party to hidden.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc

🥳

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Left some comments

🇺🇸United States hooroomoo nyc

@jessebaker do you have concrete steps to reproduce the issue on 1.x? I tried to create similar scenarios as the GIF with the components i have but i haven't been able to reproduce the issue

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc

I opened 2 new issues:
#3551343: Add type-appropriate icons for LinkedFieldBox.tsx in `ContentTemplates`
📌 Triage shape-matched DynamicPropSources: remove irrelevant ones such as any revision information Active

I am adding them to the Lower priority/need scope clarity Confirmed with product these are out of scope for 1.0 section for now... If anyone disagrees please feel free to move it. But adding them to the META so they can be tracked.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo created an issue.

🇺🇸United States hooroomoo nyc

Nice!!!

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo created an issue.

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Adding "remove feature flag" in issue title for VISIBLITY

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Video



Edge case where a valid parent suggestion also has children suggestion. In this case, we render the parent suggestion in it's own submenu with a separator below it

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc

adding at least 1 tag to this so this doesn't get lost in the weedz

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

I approved but want @jessebaker to check over my latest commit.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

📌 Follow-up for #3541037: Contextual panel flickers when linking prop to field Active is in! so changing status from postponed

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

We can close this. Can open a new issue if something else comes up.

🇺🇸United States hooroomoo nyc

Left a question

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Tested and works great! Left some comments and a question

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

When deleting a component on a page, it should not redirect user outside of the page.

So this got fixed in 📌 FE follow up to Only allow deleting Code Components if there's 0 usages Active

But looks like @lauriii is asking for

When deleting a component while editing that component, it should redirect user back to the page they were editing before.

so we don't want to redirect to /editor if you were editing the code component that was deleted

🇺🇸United States hooroomoo nyc

This is the media field I configured for the Article content type and the second code snippet is the article content template where I added the Card SDC.

// field.field.node.article.field_media.yml

uuid: 64ec6591-f4f6-4258-90a0-4f58ba742827
langcode: en
status: true
dependencies:
  config:
    - field.storage.node.field_media
    - media.type.audio
    - media.type.document
    - media.type.image
    - media.type.remote_video
    - media.type.video
    - node.type.article
id: node.article.field_media
field_name: field_media
entity_type: node
bundle: article
label: media
description: ''
required: true
translatable: false
default_value: {  }
default_value_callback: ''
settings:
  handler: 'default:media'
  handler_settings:
    target_bundles:
      audio: audio
      document: document
      image: image
      remote_video: remote_video
      video: video
    sort:
      field: _none
      direction: ASC
    auto_create: false
    auto_create_bundle: document
field_type: entity_reference

// canvas.content_template.node.article.full.yml

uuid: 4b74ff90-c411-48c7-a655-78bd64200a52
langcode: en
status: true
dependencies:
  config:
    - canvas.component.sdc.canvas_test_sdc.card
    - core.entity_view_mode.node.full
  module:
    - options
id: node.article.full
content_entity_type_id: node
content_entity_type_bundle: article
content_entity_type_view_mode: full
component_tree:
  -
    uuid: 8f152218-fdcf-467b-bd11-4c408400e5af
    component_id: sdc.canvas_test_sdc.card
    component_version: 3b105d413d26cd38
    inputs: '{"heading":"Card","content":"In a curious work, published in Paris in 1863 by Delaville Dedreux, there is a suggestion for reaching the North Pole by an aerostat.","footer":"I have a footer!","image":{"sourceType":"default-relative-url","value":{"src":"balloons.png","alt":"Hot air balloons","width":640,"height":427},"jsonSchema":{"type":"object","properties":{"src":{"type":"string","contentMediaType":"image\/*","format":"uri-reference","x-allowed-schemes":["http","https"]},"alt":{"type":"string"},"width":{"type":"integer"},"height":{"type":"integer"}},"required":["src"]},"componentId":"sdc.canvas_test_sdc.card"},"sizes":"auto 50vw","loading":"eager"}'
exposed_slots: {  }
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Tested and #6 fixed the reported issue here! So now can render the component instance form with the presence of the media field.

But I'm not able to link a required media field (even with only image media type is selected) to the image prop of my Card SDC. Is this a known issue? Or maybe a regression happened? I see it was originally fixed here: Linker does not appear for certain field widgets 🐛 `LinkedPropSource` render element does not appear for Media Library Widget Active

1. Add required media field to article content type with only image selected
2. Create an article and add an image to the media field
3. Go to canvas and open article template for the node created
4. Add Card SDC and there is no linker that appears for the image prop

🇺🇸United States hooroomoo nyc

Nice sleuthing

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

2 small things

🇺🇸United States hooroomoo nyc

@bnjmnm walked me through the MR and his reasoning and it makes sense to me. Yes global window object is side-eyed sometimes, but I think with the ComponentInstanceForm being special and unique in that it can get updated via React or AJAX, it is a valid use case to use the window object since the conventional ReactContext did not completely fulfill our needs by itself. And it is also prefixed with an underscore which is common convention for internal usage and is used in other apps including React which he pointed out.

It did make me wonder if the transforms are static, why don't we store them client-side in a static object so the ComponentInstanceForm always has access to it instead of getting it from the backend?

But after reading redux-integrated-field-widgets.md specifically 3.4.1 Defining your own transform , looks like at least one of the big reasons is for extensibility, so someone can define their own transforms in JS or PHP then the FE would have access to it since the server is the source of truth.

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Left some comments

🇺🇸United States hooroomoo nyc

hooroomoo made their first commit to this issue’s fork.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Expected API here :-)

And have it in a way where the frontend can just render the JSON directly without any re-ordering/manipulating based on @lauriii's guidelines in #25?

  {
    "id": "some-unique-id" 
    "label": "Image",
    "source": <opaque JSON object>,
    "items": [
      { "id": "some-unique-id", "label": "Changed", "source": <opaque JSON object> },
      { "id": "some-unique-id",  "label": "Created", "source": <opaque JSON object> },
      { "id": "some-unique-id", "label": "File size", "source": <opaque JSON object> }, 
    ]
  },
🇺🇸United States hooroomoo nyc

Removing needs design/needs product tag since we have a path forward now

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

hooroomoo created an issue.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

Or does that happen already and this is to catch the case where the entity was deleted but the UI hasn't been refreshed yet (or if the user visits an out of date url with the id in the params?)?

Yes this, now if a user refreshes from being on an out of date URL, it will redirect to /canvas/template/node/{bundle}/{viewMode} which will either re-route to the new suggestedEntityId or if there is no more content, to the "No content available" screen.

🇺🇸United States hooroomoo nyc

@jessebaker based on #11, I can open a follow-up for that?

Also after adding the feedback Ben left, the Settings tab stays in the panel and only the form refreshes.

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

wasn't able to get to this today

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc

test failed

🇺🇸United States hooroomoo nyc

Crazy work (very complimentary), left a comment about debounce

🇺🇸United States hooroomoo nyc
🇺🇸United States hooroomoo nyc
Production build 0.71.5 2024