lism-ui: AccordionのReact版Item.tsxを、Modalと同じLism forwardedRefの書き方に揃える

Open Beginner friendly
  • Is something wrong?
#670 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
react, typescript
Domain
developer-experience, frontend

Research direction

Read packages/lism-ui/src/components/Accordion/react/Item.tsx and compare it with packages/lism-ui/src/components/Modal/react/Modal.tsx and packages/lism-css/src/lib/getLismProps.ts. Update Accordion Item to pass its ref through Lism while preserving as, exProps, initial data-opened state, and event setup and cleanup; remove the outdated comment. Done when those behaviors remain unchanged.

Written by the indexing model from the issue text.

Description

背景

lism-ui の Accordion（React 版）の Item.tsx は、<Lism> を使わずに独自の書き方で描画しています。その理由として書かれているコメントが、今の Lism の実装と合っていません。同じ lism-ui の Modal と書き方を揃え、不要なコメントもなくします。

詳細

対象: packages/lism-ui/src/components/Accordion/react/Item.tsx

このファイルには、次のコメントがあります。

// Lism コンポーネントは ref を forwardRef していないため、React 18 互換のため動的 Tag + getLismProps で実装

このコメントに沿って、Item.tsx は次のように実装されています。

  • getLismProps() で、クラスやスタイルを自分で組み立てる
  • as で受け取ったタグ（Tag）を直接描画し、そこに ref を付ける

しかし、今の Lism は forwardedRef で ref を DOM 要素へ渡せます（packages/lism-css/src/lib/getLismProps.ts）。同じ lism-ui の Modal/react/Modal.tsx は、次のように <Lism> に ref を渡しています。

<Lism forwardedRef={ref} as="dialog" className={atts(className, 'b--modal')} ...>

そのため、コメントに書かれた理由は今は成り立たず、Item.tsx だけが遠回りな書き方のまま残っています。lism-ui の中で getLismProps() を直接呼んでいるのも、この Item.tsx だけです。

なお、このファイルは lism-cli ui add で利用者のプロジェクトへコピーされる配布コードです。

改善方法

  • Item.tsx を、Modal と同じ <Lism forwardedRef={ref} ...> の形に書き換える
  • 不要になった上記のコメントを削除する

確認事項

書き換えた後、次の点が今と変わらないことを確かめます。

  • as と exProps の扱い
  • isOpen による初期展開（data-opened の付与）
  • 開閉イベントの登録（setEvent）
  • アンマウント時の開閉イベントの解除

やらないこと

  • 今の実装を残し、コメントだけを正しい理由に書き直す案は採りません。今の書き方が必要になる理由が見当たらないためです。
Dominant language
TypeScript
Stars
283
Forks
8
Avg merge
6h 37m
Merged PRs (30d)
15

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from lism-css/core

All issues in lism-css/core

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.