🇺🇸United States @hooroomoo

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

Merge Requests

More

Recent comments

🇺🇸United States hooroomoo

2 small things

🇺🇸United States hooroomoo

@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

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

🇺🇸United States hooroomoo

Left some comments

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo
🇺🇸United States hooroomoo

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

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

🇺🇸United States hooroomoo
🇺🇸United States hooroomoo

hooroomoo created an issue.

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

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

@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
🇺🇸United States hooroomoo

wasn't able to get to this today

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

test failed

🇺🇸United States hooroomoo

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

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

Responded in https://www.drupal.org/project/canvas/issues/3547598#comment-16279558 📌 Refine API response with DynamicPropSource suggestions to provide better UX Active

🇺🇸United States hooroomoo

We can do something like this where we utilize separators and labels to differentiate between a field pointing to a field on another entity and a field of which only a subset is consumed (and this could also be a reference field!)

I mocked this up in Figma really quick using the example @wimleers gave in #11 (last code block):

So we could use a label in this case ("File") for fields for another entity and add a separator after it.

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

Added these issues relating to mapping props to field types to their own category, feel free to rename category or move them to a different one, just want to capture them in the META.

Support linking image and video field to props in a `ContentTemplate` Active
Support linking User/author field to props in a `ContentTemplate` Active
Support linking field types marked SUPPORTED from #3512433 to props in a `ContentTemplate` Active

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

Not sure if there is another MR that's supposed to happen here: 📌 Refine API response with DynamicPropSource suggestions to provide better UX Active that would be good to have before starting this MR

🇺🇸United States hooroomoo
🇺🇸United States hooroomoo

hooroomoo created an issue.

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

hooroomoo created an issue.

🇺🇸United States hooroomoo

Updated IS to specify media vs file upload image

- Video (media)
- Image (media)
- Image (file upload)

🇺🇸United States hooroomoo
🇺🇸United States hooroomoo

#3 Ok, I'll try to see if it shows up after marking required.

🇺🇸United States hooroomoo

@wim leers So in my blog content type, I have both a "Media: image" field and a "File upload: image" field.

Neither shows up in the list of suggestions for any of the components I have that have an image prop (code component with image prop, Card component SDC, Canvas test SDC for image gallery)

Images work:

Is this referring to "File upload: image" field?

🇺🇸United States hooroomoo

hooroomoo created an issue.

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

hooroomoo created an issue.

🇺🇸United States hooroomoo

hooroomoo created an issue.

🇺🇸United States hooroomoo

+ 1 for addressing the flicker to avoid stale data in a follow-up so this can get in.

🇺🇸United States hooroomoo

Thanks!

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

hooroomoo created an issue.

🇺🇸United States hooroomoo

Thanks!

🇺🇸United States hooroomoo

hooroomoo created an issue.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

updated IS to link to correct MR comment (previous link wasn't pointing to the MR comment)

🇺🇸United States hooroomoo

not currently working on this anymore so unassigning myself in case someone else wants to pick it up

🇺🇸United States hooroomoo

hooroomoo created an issue.

🇺🇸United States hooroomoo

Couldn't get to this today, un-assigning myself in case someone out of EST time wants to work on it

otherwise I'll pick it back up tomorrow

Production build 0.71.5 2024