Skip to content

feat: dasc #181 handle arrays, #196 annotate based on schema#198

Merged
kozmaadrian merged 29 commits intomainfrom
issue/196
Feb 25, 2026
Merged

feat: dasc #181 handle arrays, #196 annotate based on schema#198
kozmaadrian merged 29 commits intomainfrom
issue/196

Conversation

@kozmaadrian
Copy link
Contributor

@kozmaadrian kozmaadrian commented Feb 19, 2026

While working on #181, we realised that we need to modify a core part of the annotation mechanism described in #196.

The refactored annotation mechanism depends on the changes introduced in #181 to properly test both the UI behavior and data persistence.

The updates are split across two separate PRs:

The current PR is intended to provide a clearer view of all changes compared to the main branch.

Demo:

demo-pr-198.mov

@aem-code-sync
Copy link

aem-code-sync bot commented Feb 19, 2026

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

</div>
<p class="primitive-item-title">${item.schema.title}</p>
<sl-select name="${item.pointer}" value="${item.data ?? ''}" @change=${this.handleChange}>
<option value="" disabled>Please Select</option>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are not enforcing any values yet, a general non-selectable (disabled) option is added to ensure the UI displays a clear and informative value.


case 'null':
return null;
return schema.default ?? false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As agreed, boolean fields must default to false as a fallback value.

auniverseaway
auniverseaway previously approved these changes Feb 20, 2026
Copy link
Member

@auniverseaway auniverseaway left a comment

Choose a reason for hiding this comment

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

@kozmaadrian @mhaack at a high level this looks OK to me. Certainly fine to unblock us and move to the schema-first approach. If I start digesting the code more and have issues, I can raise PRs.

@chrischrischris would you mind give this a quick glance? I want to make sure we have SMEs available in US time zone to be able to support this feature. I can give a lot more context when needed.

Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com>
@kozmaadrian
Copy link
Contributor Author

@mhaack @chrischrischris @auniverseaway

I’ve applied the requested small change to the PR. Could one of you please help and merge it when you have a moment?

@hannessolo
Copy link
Contributor

I figured out how to make it run, but found some issues.

Screenshot 2026-02-24 at 15 30 34
  • Boolean values don't seem to work, the check box appears but doesn't add a value to the json output
  • For some reason the UI isn't displaying the property names
  • The navigation is completely empty

But maybe I just didn't set it up correctly, if you have a template I can copy instead of my from-scratch setup here that might help.

I used

https://da.live/formsref?nx=local#/hannessolo/da-playground/formstest
https://da.live/apps/schema#/hannessolo/da-playground

@mhaack
Copy link
Contributor

mhaack commented Feb 24, 2026

The boolean might be related to the storage, only "true" is stored. This is fixed by #201

The other two issues are new to me.

@hannessolo
Copy link
Contributor

Boolean values don't seem to work, the check box appears but doesn't add a value to the json output

Now fixed

For some reason the UI isn't displaying the property names

I was missing the title property in the schema

The navigation is completely empty

Same, title missing

Copy link
Contributor

@hannessolo hannessolo left a comment

Choose a reason for hiding this comment

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

I tested it and it looks fine to me to get the core functionality in, as @auniverseaway commented we can always refactor things in a new PR if there's things we don't like.

@kozmaadrian kozmaadrian merged commit a968135 into main Feb 25, 2026
3 of 4 checks passed
@kozmaadrian kozmaadrian deleted the issue/196 branch February 25, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants