Vienna
Account created on 21 January 2005, almost 21 years ago
#

Merge Requests

More

Recent comments

🇦🇹Austria fago Vienna

ok, some updates:
- I was not able to find any issues with the media-library with manual testing.
- I tried a couple of test-runs, it seems the test just became more flaky. I had 1 successful run, but usually media-library-component-instance fails with 1-2 failed tests. I try whether I can make it more reliable.

🇦🇹Austria fago Vienna

Thank you for taking a look!

of course, I'll take a closer look at the test and re-play it manually. Hopefully I can help fixing.

🇦🇹Austria fago Vienna

Got it fixed, merged!

🇦🇹Austria fago Vienna

This broke with https://git.drupalcode.org/project/custom_elements/-/commit/8fd5a2570532... - the new explicit API output which is default for new installs - like tests. Instead of enabling legacy mode, let's update the tests to work with explicit mode.

🇦🇹Austria fago Vienna

this works like a charm now! CI has same test fail as 3.x-dev + works locally with the canvas fix applied. Merged!

🇦🇹Austria fago Vienna

Merged!

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

Created 🐛 UI shows error 'can't access property "resolved", ne[r] is undefined' with extjs-components having no props Active for fixing this in canvas. Leaving this issue open for everyone to find it easily.
Note: After applying the fix in canvas you need to re-build the canvas/ui - run "npm run build" in the ui directory (for details: see canvas docs)

🇦🇹Austria fago Vienna

MR for the fix is attached

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

While it somewhat works after adding the slot markers, it does NOT work reliable. It's still runs into timing issues so sometimes works and sometimes not.

I've been testing this a lot during the last couple of days, with the the updated MR. Given that, it works reliably now!

The only remaining issue I run into is 🐛 UI sometimes show error "can't access property "resolved", ne[r] is undefined Active , what is another issue though. I do some more debugging to verify I have a good fix there and open a separate issue then.

🇦🇹Austria fago Vienna

MR is green besides some cypress E2E fails, which seem to be unrelated/flaky? tests.

🇦🇹Austria fago Vienna

Ready. "only" same failas as in 🐛 3.x-dev canvas integration test fails of today Active

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

The useEffect runs too late, the error is still happening before. The useEffect just triggers another update that makes the error go away visually. I was able to make it work by skipping this too-early triggered form-re-rendering when the selected-component is not the current component. That seems to fix it!

🇦🇹Austria fago Vienna

Somehow this is broken for extjs components that have no props, while it works for other (e.g. js) components that have no props.
Not sure why, but in the extjs-component case the effect for fetch the form does not trigger.

It might be that React is batching updates for:
- Line 393: setDynamicStaticCardQueryString('')
- Line 412: setDynamicStaticCardQueryString(queryString)
so that RTK Query reads the state BEFORE the second update applies

I tried removing the first setDynamicStaticCardQueryString('') since it gets overwritten anyway later, but that did only partially solve. The problem was still triggered often, it seems there is a race condition somewhere.

So we need to reliably clear the the form content when the component is changed -> added a useEffect() for that. That solves it - I did not manage to break it anymore then! Anyway, we need to open a canvas issue here.

🇦🇹Austria fago Vienna

Here is a backtrace when running in dev-mode:

Uncaught TypeError: can't access property "resolved", inputAndUiData.model[currentComponent] is undefined
    InputBehaviorsComponentPropsForm InputBehaviorsComponentPropsForm.tsx:282
    InputBehaviorsWrapper inputBehaviors.tsx:313
    React 18
    Redux 9
    rtkQueryErrorHandler rtkQuery-error.ts:123
    Redux 2
    undoRedoActionIdMiddleware store.ts:208
    Redux 10
    createImmutableStateInvariantMiddleware Immutable
    createActionCreatorInvariantMiddleware Redux
    updateSelectionInRedux useComponentSelection.ts:154
    handleComponentSelection useComponentSelection.ts:201
    handleComponentClick ComponentOverlay.tsx:134
    React 27
    workLoop scheduler.development.js:266
    flushWork scheduler.development.js:239
    performWorkUntilDeadline scheduler.development.js:533
    js scheduler.development.js:571
    js scheduler.development.js:633
    __require chunk-OCBYBPSH.js:15
    js index.js:6
    __require chunk-OCBYBPSH.js:15
    React 2
    __require chunk-OCBYBPSH.js:15
    js

It seems to process the input-form of the *previous* component, then breaks. Because of that the bug appears only when navigating FROM certain components to it.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

New installs of custom elements module are now using the explicit JSON output. We need to enable legacy mode or start using it. -> Adapted tests to start using it.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

oops, of course - it's alright. I tested the update patch as well, it still works fine. Thus, this is ready!

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

Let's fix this then.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

Thanks! However, both versions of the code seem to use the same SensorResultInterface constants?

🇦🇹Austria fago Vienna

The MR correctly addresses the issue also.

🇦🇹Austria fago Vienna

MR seems all good. Uploading a patch for use with composer-patches also + giving it a test now.

🇦🇹Austria fago Vienna

The old constants are deprecate, but still available. They should still work.
See https://git.drupalcode.org/project/drupal/-/commit/76dcf5a207d8f96c47508... and https://www.drupal.org/node/3410939

I can report that the patch works, thanks!
However, for merging it I'd think it would be best to make it work in a backwards-compatible way, e.g. using the deprecation helper. . Thus setting needs work for that.

🇦🇹Austria fago Vienna

I also ran into this. Testing the fix.

However, this patch will only work with Drupal 11.2+. What made the old constants go away? The should be still defined. Maybe something stopped including the install.inc file where the constants are defined?

🇦🇹Austria fago Vienna

attended

🇦🇹Austria fago Vienna

Adding patch for usage with composer patches also.

🇦🇹Austria fago Vienna

fix capitalization

🇦🇹Austria fago Vienna

moving to related item in APIs

🇦🇹Austria fago Vienna

moved into frontend libraries sectoin

🇦🇹Austria fago Vienna

improve summary

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

better structure

🇦🇹Austria fago Vienna

The page is in-complete and unlikely to be finished ever. Implementation would differ a lot by frontend choice, it's better to create separate ones depending on the frontend.

🇦🇹Austria fago Vienna

I successfully tested this also works in a Drupal 10 install.

🇦🇹Austria fago Vienna

turns out, we overwriting the ::preview submit handler, what seems fragile. Instead of replacing the logic, we should append it. Create MR.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

Besides that, there is also an issue tests in Drupal 10. So this needs work, no triage needed any more.
Attaching a patch of the latest changes as well, for usage with composer-patches.

🇦🇹Austria fago Vienna

ok, done so - I hope this helps. Merged the improvements.

🇦🇹Austria fago Vienna

thank you for your report. It seems like our documentation needs some improvements, I'll take care of that.

🇦🇹Austria fago Vienna

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

🇦🇹Austria fago Vienna

same failure as in 3.x-dev - so change should be good
opened 🐛 3.x-dev canvas integration test fails of today Active

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

all green again

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

Found the issue:

Changed Behavior in Drupal 11.x-dev:

Before (Drupal 11.2.5):
- When Layout Builder was enabled on a core entity view display, field components remained in $display->content
- $display->getComponents() returned all configured fields
- custom_elements could copy these components when auto-generating CE displays

After (Drupal 11.x-dev, commit 60bb1a2b7e):
- When Layout Builder is enabled, field components are immediately removed from $display->content and moved to $display->hidden
- $display->getComponents() returns an empty array (since it just returns $this->content)
- custom_elements auto-generation finds no components to copy, resulting in fields being null

This change in behavior is actually acceptable and just a slight change in UX - as long as people have saved their custom-element entity displays. I'd expect that to be the case anyway, but we can document this change in a change-notice to play save. Generally, it could be argued that having an empty list of fields is a good default IF layout builder is used.

TL/DR: I think the status quo with behavior being changed with the core upgrade is fine and we just need to make tests check for that. In th end it's a minor-version update of core and some small changes like this are expected there. There is no change when config is exported properly anyway.

-> Let's use something like https://www.drupal.org/node/3379306

🇦🇹Austria fago Vienna

One more though. Maybe we should do it similar to Views, and simply have a preview-area that is always there + a button to make it load? Since we need the button to make it load anyway, the fieldset wrapper seems unnecessary. Then we could add a drop-down to select the desired preview-provider also. --> for the dropdown we need not use the plugins but get the list of provider-services from our provider-resolver.

🇦🇹Austria fago Vienna

The MR successfully addresses the remaining issues.

🇦🇹Austria fago Vienna

> Each slot gets rendered via a preview again, what is not working out in this case.

Actually, it's working out. It's just missing the tag-prefix, what cause issues.

🇦🇹Austria fago Vienna

Another issue is handling slots in previews.

Current approach for the nuxt-preview is only working well with canvas, not with regular components. Each slot gets rendered via a preview again, what is not working out in this case.

🇦🇹Austria fago Vienna

After some debugging, I figured the problem is actually on the nuxt side of things.
This commit fixed it.

However, after fixing it I found another issue with the component-preview. It does not respect the nuxtjs-drupal-ce component preview, since it's simply using h() and not renderCustomElements() helper.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

MR looks good. However, when testing changes I do not see the button?

The preview is refreshed when changing formatter settings, but when e.g. renaming a field in output, I don't have a way to update the preview still? So something seems not right here.

🇦🇹Austria fago Vienna

Feedback on this change was overall positive, so let's move on and merge this for easier testing.

Changes are backwards compatible, existing install will stay on legacy format. Changes come with a new settings form. New installs will have explicit format enabled by default.

🇦🇹Austria fago Vienna

Merged. Composer verifies module versions match.

🇦🇹Austria fago Vienna

pushed fix

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

frontend PR https://github.com/drunomics/nuxtjs-drupal-ce/pull/402 is ready for review. it's backward compatible

🇦🇹Austria fago Vienna

Since our custom element class uses attributes terminology, not props, the question is what we should use here.

Overall usage:
- Custom Elements spec: "attributes" (string-only) vs "properties" (any type)
- React: "props"
- Vue: "props" (component data) vs "attrs" passed (HTML attributes)

More important, the Web Components Terminology Distinction

Attributes (HTML):
- String-only values
- Set via HTML:
- Retrieved via: element.getAttribute('title')

Properties (JavaScript):
- Any JavaScript type (objects, arrays, etc.)
- Set via JavaScript: element.imageData = {src: '...', alt: '...'}
- Retrieved via: element.imageData

--> in our case, the values may be objects, like imagedata, not only strings. So I think going with props instead of attributes makes sense + it fits the JS-world well.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

ready, let's merge when CI is happy

🇦🇹Austria fago Vienna

Merged!

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

added it as second MR now

🇦🇹Austria fago Vienna

I've got a new version of the changes without the dependencies, working fine for me as well. Unfortunately gitlab is down atm, so cannot push it, so attaching a full patch meanwhile instead.

🇦🇹Austria fago Vienna

While looking into why this was not fixing the issue, I figured out what is the real problem: Canvas ExtJS simply missed the slot html comments... ! I was not aware there are both component and slot comment, while the component-comments worked, slot comments were missing and cause the issue. I've fixed that in 🐛 Annotations for child components within slots are lost Active now - and this makes things working with and without (!) this fix! SRY for missing that before :-/

In any case, this fix seems to be a reasonable addition though. Honestly, I must say I don't know why it works without, possibly it depends on a lucky timing.

Looking at the code, however I'm not sure the added dependency on the update functions is right. In my understanding, it would lead to the useComponentHtmlMap's useEffect to re-render and thus trigger a re-creation of the mutation observer on every change. I think simply using the observer to trigger the updates should be good enough + the improvement of the equality check makes sense!

🇦🇹Austria fago Vienna

it turns out, this fixes the issue! So it seems the missing slot-comments were really the main problem

🇦🇹Austria fago Vienna
🇦🇹Austria fago Vienna

oh, I just figured that the canvas-start/stop comments are there but NOT the canvas-slot-start-UUID/SLOTNAME comments!

So we need to fix this first.

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

thx, that looks mostly great now! I've found case where I think we should throw an exception though, see MR. Could you check that?

🇦🇹Austria fago Vienna

fago created an issue.

🇦🇹Austria fago Vienna

Adding an update with the material and demo from the drupalcon session about this:

🇦🇹Austria fago Vienna

As clarified in the mentioned ticket, we are moving on with the new entity-generic version developed by drunomics as 3.x.
While the functionality is similar, this is a completely different module and thus there is NO upgrade path. Remove old views plugins and configure the new ones instead.

🇦🇹Austria fago Vienna

ok, many weeks have passed, much more than the required 2 weeks, + we get kars-t's approval, so I'm moving on with this now.

I'm marking 2.x as unsupported and starting 3.x with the entity-generic 3.x version as proposed.
If you are interested in maintaining 2.x please reach out to me.

Production build 0.71.5 2024