Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

フォームの表示 #4

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

sititou70
Copy link

No description provided.

src/index.ts Outdated
<input type="${item.type}" id="${x.label}" name="${item.name}" value="${x.value}">
<label for="${x.label}">${x.label}</label>`
)
.reduce((s, x) => s + x);
Copy link
Collaborator

Choose a reason for hiding this comment

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

文字列の配列を連結する際は .join("") が使えたりします。

Copy link
Author

Choose a reason for hiding this comment

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

あ、たしかにそうですね

const inputs_html = item.values
.map(
(x) => `
<input type="${item.type}" id="${x.label}" name="${item.name}" value="${x.value}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

name属性の扱いについて、 radio と checkbox で扱いが同じでよいか?ということを考えてみるとよいと思います。
型としては両方 MultipleValueInputItems として扱ってよいのですが、出力されるHTMLがこれで問題ないか( = チェックボックスのnameが全valuesで同じ値になってしまっている) 点は要注意ですね

Copy link
Author

Choose a reason for hiding this comment

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

たしかに…気づかなかったです…!

Copy link

Choose a reason for hiding this comment

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

これ、私も同じことをしていたのですが、 type=checkbox で同一の name 属性を持つものが複数存在することは問題ないのではないか、と思ったのですが、何か問題があるのでしょうか……?

type SingleValueInputItems =
| SingleValueInputItem<"text">
| SingleValueInputItem<"email">
| SingleValueInputItem<"tel">;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Genericsを使ってコード量を上手に削減していますね、とてもよい工夫だと思います 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants