List things related to a resource with a custom display

[jg10-mastodon-social]

Partly addresses #43

• List things linked with a particular predicate (rel)
• Specify a template to use for each thing
• Hydrated list is valid RDFa - intended to be compatible with draft spec https://jg10.solidcommunity.net/open-with/rdf-ui-binding.html#subject-connected-list
• Thing provides methods to extract specific rel
• Tests have been written
  • all new code is covered by unit tests
  • the happy path of a new feature is covered by an end-to-end test integration test
    • Integration test similar to https://github.com/pod-os/PodOS/blob/main/elements/src/components/pos-label/pos-label.integration.spec.tsx
  • manual explorative tests have been performed
• all dependencies are updated to the latest patch version at minimum
• the CI pipeline passes
• documentation is up-to-date
  • TSDoc style comments on important functions, properties and events
  • stories for new PodOS elements have been added to storybook
  • Readme.md files of PodOS elements have been re-generated
  • architectural decisions are documented as an ADR
  • Changelogs are updated according to
    Keep a Changelog

This PR descoped to exclude rev, given this will involve changes very similar to rel. Covering rev in a separate PR will hopefully make both PRs easier to review.

[jg10-mastodon-social]

The reason for this is that the component will provide resources to each child based on their index. Compared to #43 (comment) this gets rid of the intermediate pos-resource. However, it will still not be possible to have li nested directly within ul, given that pos-list lies between them.

[jg10-mastodon-social]

One option would be to allow pos-list to be used as a customized built-in element, i.e. <ul is="pos-list"></ul>. I would need to look into whether/how stenciljs supports this.

Update: stenciljs does not support customized built-in elements stenciljs/core#4089 (comment)

[angelo-v]

What about assigning role="list"?

[jg10-mastodon-social]

The issue I had in mind is that the only permitted parents for li are ul and ol (or menu). So if a dashboard user cares about correct HTML then I suppose they could use <div role="listitem" style="display:list-item"> I hadn't thought of that.

[jg10-mastodon-social]

I've currently ignored correct HTML for this PR. In storybook I've used bare lis. Let me know if it's an issue.

[angelo-v]

I think we should care for correct HTML and accessibility. Are you sure it would be invalid? As I understand this answer it would not

[jg10-mastodon-social]

I wasn't aware of that, and indeed I tried the pos-list-composition storybook example in https://websiteaccessibilitychecker.com/checker/index.php and it also passes.

[angelo-v]

In my understanding pos-list should have role "list" and each child role "listitem" by default without additional effort by the dashboard author. pos-list is a list by definition and the created children are it's items

[angelo-v]

This is what I get in storybook adding role="listitem" to pos-resource and <Host role="list"> arround the elems

[jg10-mastodon-social]

I've marked as ready for review because I'm not sure what e2e test should be included (is the integration test enough?), and I think we can maybe descope and implement rev in a separate PR - this one is probably complex enough as it is?

[angelo-v]

Good work so far! I have several comments and questions, but many of them are minor, I think this goes into a good direction.

[angelo-v]

Why ware we doing this instead of wrapping each child in a pos-resource?

[jg10-mastodon-social]

Current rationale is described at #119 (comment)
I may have made life hard for myself - I suppose the main motivation is that it's more intuitive for a dashboard creator to reason about what will will happen with the provided template - having a hidden pos-resource inserted is a big of a surprise.

In principle it also gives the list more control over the (re-)rendering of its children, but that's a hypothetical advantage at this point, until we tackle reactive rendering.
I haven't yet experimented with how we would deal with list mutations, but I expect we will eventually want to be able to allow deleting, adding, and reordering list elements.

Edit: I should also add that I have been considering changes to the produced HTML to be breaking changes, so I didn't want to start with pos-resource and change this later.

[angelo-v]

Hmm, have to think about this. I wonder if we are going to re-implement a lot of pos-resource here, especially if we want to have eager loading eventually. I would not consider changes in the HTML a breaking change as long as the observable behaviour does not change

[angelo-v]

Wouldn't having an explicit pos-resource be actually less surprise, given that e.g. pos-label currently relies on having a parent pos-resource as a point of reference. The current implementation of pos-list changes this and the inner pos-label now suddenly is referencing a kind of "hidden" pos-resource implicitly.

[jg10-mastodon-social]

Given I'm getting rid of provideQueue, I'll switch to using pos-resource for now.

I suppose from a dashboard creator point of view I consider generated HTML as an observable behaviour, but as you note elsewhere, we are still on v0.x

[jg10-mastodon-social]

I've refactored to use pos-resource. See what you think :-)

[angelo-v]

all dependencies are updated to the latest patch version at minimum

You don't need to bother with this in the PR, I will do this after merging before releasing a new version

[angelo-v]

I am having an issue using this within another stencil component. If I paste an example directly in index.html it works, but the same example fails when I e.g. use it in pos-app-browser (just replacecing the regular pos-app content with the pos-list example).

In the latter case it fails, because it does not recognize the template child (childElementCount == 0). Don't know why yet.

[angelo-v]

It seems to be related to what is described here: https://stackoverflow.com/a/70778523

If I pass the content to <template innerHTML={"..."} instead of the template body then it works!

[angelo-v]

I managed to show my bookmarks using pos-list by the way, which is really cool!

<pos-app>
      <pos-resource uri="https://angelo.veltens.org/public/bookmarks">
        <pos-list rel="http://purl.org/dc/terms/references">
          <template>
            <article>
              <h2>
                <pos-label></pos-label>
              </h2>
              <pos-value predicate="http://www.w3.org/2002/01/bookmark#recalls"></pos-value>
            </article>
          </template>
        </pos-list>
      </pos-resource>
    </pos-app>

What is unfortunately still not possible with PodOS is actually linking to the value because I cannot use pos-value in an <a href for example. Not sure if we have an issue for this yet see #123

[jg10-mastodon-social]

If I pass the content to <template innerHTML={"..."} instead of the template body then it works!

That's frustrating. Is that ok as a solution, or would you like to explore an approach that doesn't use the template tag?

I would expect that stencil components would very rarely use pos-list - given they are written in JS they can just use Thing's methods directly and create their own list.

[angelo-v]

That's frustrating. Is that ok as a solution, or would you like to explore an approach that doesn't use the template tag?

Nah, I think template is the right approach, is just weired with stencil, I guess we can live with that.

I would expect that stencil components would very rarely use pos-list - given they are written in JS they can just use Thing's methods directly and create their own list.

Yes indeed, though I think the issue is not only with stencil but with everything that creates template programmatically, e.g. also with React and other frameworks (would need to try out seems to be https://stackoverflow.com/a/42845196). But you could argue using those frameworks you are / have to be aware of this behavoiur, it is not unique to PodOS but html template in general.

[jg10-mastodon-social]

With the switch to pos-resource it turns out I need to address loading of resources (incl. #97 for lists) in this PR because pos-resource by default does fetch.

I've run into two problems: I can't get pos-resource to not fetch when testing, and even when it does fetch, os.fetch is being called twice, the first time with an undefined uri.
https://github.com/pod-os/PodOS/actions/runs/15671398762/job/44143056979?pr=119#step:7:438

Storybook seems to work fine. I'm guessing I've made a mistake in mocking, but maybe it's in how I call pos-resource in pos-list?

On my system I also get warnings:

    console.warn
      @Prop() "uri" on <pos-resource> is immutable but was modified from within the component.
      More information: https://stenciljs.com/docs/properties#prop-mutability

      at consoleDevWarn (node_modules/@stencil/core/internal/testing/index.js:162:11)
      at PosResource.set (node_modules/@stencil/core/internal/testing/index.js:2882:17)
      at PosResource.set (node_modules/@stencil/core/internal/testing/index.js:2895:26)
      at setterSetVal (node_modules/@stencil/core/internal/testing/index.js:2921:48)
      at node_modules/@stencil/core/internal/testing/index.js:2927:49

    console.warn
      @Prop() "lazy" on <pos-resource> is immutable but was modified from within the component.
      More information: https://stenciljs.com/docs/properties#prop-mutability

      at consoleDevWarn (node_modules/@stencil/core/internal/testing/index.js:162:11)
      at PosResource.set (node_modules/@stencil/core/internal/testing/index.js:2882:17)
      at PosResource.set [as lazy] (node_modules/@stencil/core/internal/testing/index.js:2895:26)
      at setterSetVal (node_modules/@stencil/core/internal/testing/index.js:2921:48)
      at node_modules/@stencil/core/internal/testing/index.js:2927:49

[repeated warnings elided]

  ● pos-list › does not fetch resources by default (if fetch attribute is not present)

    expect(received).toHaveLength(expected)

    Expected length: 0
    Received length: 4
    Received array:  [[undefined], [undefined], ["https://video.test/video-1"], ["https://video.test/video-2"]]

       96 |     });
       97 |
    >  98 |     expect(os.fetch.mock.calls).toHaveLength(0);
          |                                 ^
       99 |   });
      100 |
      101 |   it('fetch resources if fetch attribute is present', async () => {

      at Object.<anonymous> (src/components/pos-list/pos-list.integration.spec.tsx:98:33)

  ● pos-list › fetch resources if fetch attribute is present

    expect(received).toHaveLength(expected)

    Expected length: 2
    Received length: 4
    Received array:  [[undefined], [undefined], ["https://video.test/video-1"], ["https://video.test/video-2"]]

      132 |     });
      133 |
    > 134 |     expect(os.fetch.mock.calls).toHaveLength(2);
          |                                 ^
      135 |     expect(os.fetch.mock.calls).toEqual([['https://video.test/video-1'], ['https://video.test/video-2']]);
      136 |   });
      137 | });

      at Object.<anonymous> (src/components/pos-list/pos-list.integration.spec.tsx:134:33)

Edit: I've pushed an extra commit that implements the fetch attribute + unit tests - but this doesn't address the issue in this integration test

[angelo-v]

This could simple be HTMLElement

[angelo-v]

Please keep the filename convention *.integration.spec.tsx. Name could be e.g. pos-list-fetch.integration.spec.tsx

[angelo-v]

Let's not repeat the querySelectorAll here, but extract the result in a variable

[angelo-v]

page is unused

[angelo-v]

same remark about repetition here