Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial support for appearances, body classes #127

Merged
merged 6 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 81 additions & 8 deletions packages/xforms-engine/src/XFormDOM.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { XFORMS_NAMESPACE_URI } from '@getodk/common/constants/xmlns.ts';
import { XFORMS_NAMESPACE_URI, XMLNS_NAMESPACE_URI } from '@getodk/common/constants/xmlns.ts';
import { XFormsXPathEvaluator } from '@getodk/xpath';

const domParser = new DOMParser();
Expand Down Expand Up @@ -41,7 +41,83 @@ const normalizeBodyRefNodesetAttributes = (body: Element): void => {
}
};

const normalizeRepeatGroups = (xformDocument: XMLDocument, body: Element): void => {
const normalizeRepeatGroupAttributes = (group: Element, repeat: Element): void => {
for (const groupAttribute of group.attributes) {
const { localName, namespaceURI, nodeName, value } = groupAttribute;

if (
// Don't propagate namespace declarations (which appear as attributes in
// the browser/XML DOM, either named `xmlns` or with an `xmlns` prefix,
// always in the XMLNS namespace).
namespaceURI === XMLNS_NAMESPACE_URI ||
// Don't propagate `ref`, it has been normalized as `nodeset` on the
// repeat element.
localName === 'ref' ||
// TODO: this accommodates tests of this normalization process, where
// certain nodes of interest are given an `id` attribute, and looked up
// for the purpose of asserting what was normalized about them. It's
// unclear if there's a generally expected behavior around the attribute.
localName === 'id'
) {
continue;
}

// TODO: The `appearance` attribute is propagated from
// `<group appearance><repeat>` to `<repeat appearance>`. But we presently
// bail if both elements define the attribute.
//
// The spec is clear that the attribute is only supported on `<group>` and
// control elements, which would suggest it should not be present on a
// `<repeat>` element directly. But many form fixtures (in e.g. Enketo)
// do have `<repeat apperance>`.
//
// It may be reasonable to relax this by:
//
// - Detecting if they share the same appearances, treated as a no-op.
//
// - Assume they're both meant to apply, and concatenate.
if (
localName === 'appearance' &&
namespaceURI === XFORMS_NAMESPACE_URI &&
repeat.hasAttribute(localName)
) {
const ref = group.getAttribute('ref');

throw new Error(
`Failed to normalize conflicting "appearances" attribute of group/repeat "${ref}"`
);
}

repeat.setAttributeNS(namespaceURI, nodeName, value);
}
};

const normalizeRepeatGroupLabel = (group: Element, repeat: Element): void => {
const groupLabel = Array.from(group.children).find((child) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ should we throw an error if there are more than one label nodes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably warn, and probably everywhere we look for 0-1 nodes (all label lookups, hints also come to mind). My understanding of the spec is that only one is “properly supported”, which to me strongly implies ignoring >1.

Feels like a good issue to file, and/or category of things to add to a general discussion of how we’ll convey errors/warnings/notices about form definitions to clients.

Copy link
Member

Choose a reason for hiding this comment

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

Added to #80

return child.localName === 'label';
});

if (groupLabel == null) {
return;
}

const repeatLabel = groupLabel.cloneNode(true) as Element;

repeatLabel.setAttribute('form-definition-source', 'repeat-group');

repeat.prepend(repeatLabel);

groupLabel.remove();
};

const unwrapRepeatGroup = (group: Element, repeat: Element): void => {
normalizeRepeatGroupAttributes(group, repeat);
normalizeRepeatGroupLabel(group, repeat);

group.replaceWith(repeat);
};

const normalizeRepeatGroups = (body: Element): void => {
const repeats = body.querySelectorAll('repeat');

for (const repeat of repeats) {
Expand All @@ -63,11 +139,8 @@ const normalizeRepeatGroups = (xformDocument: XMLDocument, body: Element): void
}
}

if (group == null) {
group = xformDocument.createElementNS(XFORMS_NAMESPACE_URI, 'group');
group.setAttribute('ref', repeatNodeset);
repeat.before(group);
group.append(repeat);
if (group != null) {
unwrapRepeatGroup(group, repeat);
}
}
};
Expand Down Expand Up @@ -113,7 +186,7 @@ const parseNormalizedXForm = (
normalizedXML = sourceXML;
} else {
normalizeBodyRefNodesetAttributes(body);
normalizeRepeatGroups(xformDocument, body);
normalizeRepeatGroups(body);

normalizedXML = html.outerHTML;
}
Expand Down
56 changes: 41 additions & 15 deletions packages/xforms-engine/test/XFormDOM.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { XFORMS_NAMESPACE_URI } from '@getodk/common/constants/xmlns.ts';
import { XHTML_NAMESPACE_URI } from '@getodk/common/constants/xmlns.ts';
import type { HtmlXFormsElement } from '@getodk/common/test/fixtures/xform-dsl/HtmlXFormsElement.ts';
import {
body,
Expand All @@ -25,6 +25,10 @@ describe('XFormDOM', () => {
mainInstance(
// prettier-ignore
t('root id="bind-types"',
t('grp',
// prettier-ignore
t('g1')
),
t('rep',
// prettier-ignore
t('a'),
Expand Down Expand Up @@ -61,11 +65,16 @@ describe('XFormDOM', () => {
),
body(
t(
'group id="group-nodeset" nodeset="/root/rep" group-containing-repeat=""',
'group id="group-nodeset" nodeset="/root/grp"',
// prettier-ignore
t('input id="input-nodeset" nodeset="/root/grp/g1"')
),
t(
'group id="repeat-group" nodeset="/root/rep" group-containing-repeat=""',
t(
'repeat id="repeat-ref" ref="/root/rep" repeat-contained-by-group=""',
// prettier-ignore
t('input id="input-nodeset" nodeset="/root/rep/a" grouped-repeat-child=""'),
t('input nodeset="/root/rep/a" grouped-repeat-child=""'),
t(
'select1 id="select-nodeset" nodeset="/root/rep/b" grouped-repeat-child=""',
t(
Expand Down Expand Up @@ -101,7 +110,7 @@ describe('XFormDOM', () => {
id: 'group-nodeset',
expected: {
nodeset: null,
ref: '/root/rep',
ref: '/root/grp',
},
},
{
Expand All @@ -119,7 +128,7 @@ describe('XFormDOM', () => {
id: 'input-nodeset',
expected: {
nodeset: null,
ref: '/root/rep/a',
ref: '/root/grp/g1',
},
},
{
Expand Down Expand Up @@ -147,23 +156,40 @@ describe('XFormDOM', () => {
expect(element.getAttribute('ref')).toBe(expected.ref);
});

it('wraps an ungrouped <repeat> with a <group> with the same nodeset reference', () => {
it('unwraps a <group ref><repeat nodeset> pair', () => {
const repeatElement = xformDOM.xformDocument.getElementById('repeat-ref')!;

expect(repeatElement).not.toBeNull();

const parent = repeatElement.parentElement!;

expect(parent.namespaceURI).toBe(XHTML_NAMESPACE_URI);
expect(parent.localName).toBe('body');

const repeatGroup = xformDOM.xformDocument.getElementById('repeat-group');

expect(repeatGroup).toBeNull();
});

it('does not unwrap an ungrouped <repeat> with a <group> with the same nodeset reference', () => {
const repeat = xformDOM.xformDocument.getElementById('ungrouped-repeat')!;
const parent = repeat.parentElement!;

expect(parent.namespaceURI).toBe(XFORMS_NAMESPACE_URI);
expect(parent.localName).toBe('group');
expect(parent.getAttribute('ref')).toBe(repeat.getAttribute('nodeset'));
expect(parent.namespaceURI).toBe(XHTML_NAMESPACE_URI);
expect(parent.localName).toBe('body');
});

it('wraps a <repeat> within a <group> referencing a different nodeset into a new <group> with its own nodeset reference', () => {
it('normalizes a <repeat ref> to use a `nodeset` attribute', () => {
const repeat = xformDOM.xformDocument.getElementById('repeat-ref')!;

expect(repeat.getAttribute('ref')).toBeNull();
expect(repeat.getAttribute('nodeset')).toBe('/root/rep');
});

it('does not unwrap a <repeat> within a <group> referencing a different nodeset', () => {
const unrelatedGroup = xformDOM.xformDocument.getElementById('unrelated-group')!;
const repeat = xformDOM.xformDocument.getElementById('unrelated-grouped-repeat')!;
const parent = repeat.parentElement!;

expect(parent.namespaceURI).toBe(XFORMS_NAMESPACE_URI);
expect(parent.localName).toBe('group');
expect(parent.getAttribute('ref')).toBe(repeat.getAttribute('nodeset'));
expect(parent.parentElement).toBe(unrelatedGroup);
expect(repeat.parentElement).toBe(unrelatedGroup);
});
});