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

Update readme #264

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Update readme #264

merged 2 commits into from
Dec 17, 2024

Conversation

lognaturel
Copy link
Member

Adds a link to the hosted preview and an explicit command to run the preview locally.

Updates the feature matrix. I know we expect to update how this is presented at some point but I find it helpful to keep it up to date for now pending that rework.

Copy link

changeset-bot bot commented Dec 13, 2024

⚠️ No Changeset found

Latest commit: 920b667

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@eyelidlessness eyelidlessness left a comment

Choose a reason for hiding this comment

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

Looks good. My inline comments are primarily concerned with the additive changes and their implications for using the feature matrix to inform dev work we discussed earlier. I don't think there's anything to address here, unless we think counter should be either marked checked or in progress.

"note": "✅",
"select_one": "✅",
"select_multiple": "✅",
"select_*_from_file": "✅",
Copy link
Member

Choose a reason for hiding this comment

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

Since it appears we missed this before, I'm concerned that we've missed others. Not the biggest deal if we're updating it as we go, but definitely doesn't make me feel very confident in using this as a basis for programmatic form feature support detection.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it was put as part of secondary instance (external choice file) in another section. I went ahead and removed that in a new commit since I think this will be more useful for users.

The XLSForm template is intended to be authoritative but it doesn't include everything that's here because it's not all relevant to XLSForm authoring. I find this format and content pretty useful but let's definitely discuss other ideas you have!

Copy link
Member

Choose a reason for hiding this comment

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

I think using the XLSForm template might be a great starting point for programmatic concerns, and would probably help narrow down cross referencing with this as a companion source of truth.

@@ -75,7 +76,8 @@
"field-list": "✅",
"label": "✅",
"list-nolabel": "✅",
"list": "✅"
"list": "✅",
"counter": ""
Copy link
Member

Choose a reason for hiding this comment

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

I had never heard of this appearance before today—first in the meeting doc about design revisions for numeric inputs, now here. I don't see it in the XLSForm Docs, but from the meeting doc it sounds like the expectation is implemented (at least partly? increment/decrement buttons). Although I guess we'd want to explicitly account for (== implement it) it so we don't regress if we decide to limit that presentation for inputs without a counter appearance. I'm fine with leaving it unchecked here if that's the intent, but wanted to note my surprise at its addition.

I also think this emphasizes my caution about treating this as a starting point for building out some sort of programmatic form feature support detection. In this case, it isn't just a gap in what we're tracking. It also implicates cross-referencing at least one other project beyond the XLSForm Docs. I see it was added relatively recently, but now I also see other appearances there which aren't in the XLSForm Docs and weren't added as recently.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was in the originalinteger issue and I probably should have at least mentioned it in review: #251 I think we need to figure out the design for the no-appearance question type and then go from there.

The authoritative sources of appearances are the XLSForm template and the ODK docs.

If you do see something missing from somewhere, please file an issue!

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I totally glazed over it in #251! I saw the reference to Enketo's presentation and probably stopped there. (Likely because the increment/decrement presentation is a de facto default on the web, as it's the default presentation for <input type="number"> in many cases.)

@lognaturel lognaturel merged commit 6b2ad27 into main Dec 17, 2024
74 checks passed
@lognaturel lognaturel deleted the readme branch December 17, 2024 22:17
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.

2 participants