Skip to content

Conversation

@arafubeatbox
Copy link
Contributor

@arafubeatbox arafubeatbox commented Dec 21, 2025

What

app client components (AuthorInfo, CreateTemplateModal, CustomNavigation, DeleteBookmarkFolderModal, EmptyTrashModal, GrantedGroupsInheritanceSelectModal, Maintenance, PageHistory, Presentation, PutbackPageModal, RecentActivity, RecentCreated, RevisionComparer, ShortcutsModal, StaffCredit, TemplateModal) の biome 移行

biome による自動修正で解決できなかった差分

https://github.com/growilabs/growi/pull/10635/files/382923617a69a4d8a4a00215f51128b53b604eba..3daae17dcb6e42bc6eb84fce1f3db8e15353890b

動作確認

  • テンプレート選択モーダルが正常に表示されることを確認。
  • ゴミ箱からページを戻す操作をするモーダルが正常に表示されることを確認。
  • ゴミ箱を空にする操作をするモーダルが正常に表示されることを確認。
  • revision 履歴モーダルの比較テーブルが正常に表示されることを確認。
  • テンプレート作成モーダルが正常に表示されることを確認。
  • ブックマークフォルダ削除モーダルが正常に表示されることを確認。

task

https://redmine.weseek.co.jp/issues/176216

@arafubeatbox arafubeatbox marked this pull request as ready for review December 22, 2025 15:33
@arafubeatbox arafubeatbox requested a review from Copilot December 22, 2025 15:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates client components to Biome for linting and formatting, removing them from the ignore list in biome.json. The migration includes components such as AuthorInfo, CreateTemplateModal, CustomNavigation, DeleteBookmarkFolderModal, EmptyTrashModal, GrantedGroupsInheritanceSelectModal, Maintenance, PageHistory, Presentation, PutbackPageModal, RecentActivity, RecentCreated, RevisionComparer, ShortcutsModal, StaffCredit, and TemplateModal.

Key changes:

  • Removed component paths from Biome ignore list in biome.json
  • Applied automatic formatting fixes (import ordering, spacing, formatting)
  • Added corresponding ESLint ignores for these components in .eslintrc.js

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.

File Description
biome.json Removed ignored component directories to enable Biome linting
apps/app/.eslintrc.js Added ESLint ignores for newly Biome-managed components
apps/app/src/client/components/*/**.tsx Applied Biome formatting (import reordering, spacing, code style)
Comments suppressed due to low confidence (1)

apps/app/src/client/components/PageHistory/PageRevisionTable.tsx:183

  • Radio input is missing an associated label. The removed label elements should be replaced with proper labels or aria-label attributes for accessibility.
          )}
        </td>
        <td className="col-2">

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

);

let SelectedNav;
let SelectedNav: (props) => JSX.Element;
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The parameter type for the function signature is missing. Specify an explicit type for props instead of using implicit any.

Copilot uses AI. Check for mistakes.
Comment on lines 196 to 197
)}
</td>
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

Radio input is missing an associated label. The removed label elements should be replaced with proper labels or aria-label attributes for accessibility.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

元々ラベルの中身がなく、表示されていない (テーブル内の radio ボタンなので、テーブルのヘッダーがラベル

<button
type="button"
className={`list-group-item list-group-item-action ${isSelected ? 'active' : ''}`}
onClick={onClick}
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

Button lacks accessible text or aria-label. Add an aria-label attribute to provide context for screen readers since the button's purpose may not be clear from the visual content alone.

Suggested change
onClick={onClick}
onClick={onClick}
aria-label={`Select template "${localizedTemplate.title}"`}

Copilot uses AI. Check for mistakes.
<button
type="button"
className="text-center staff-credit-content btn btn-link p-0 border-0"
onClick={contentsClickedHandler}
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

Button lacks accessible text or aria-label. The button contains only visual content; add an aria-label to describe its purpose for screen readers.

Suggested change
onClick={contentsClickedHandler}
onClick={contentsClickedHandler}
aria-label="View and scroll GROWI contributors credits"

Copilot uses AI. Check for mistakes.
Base automatically changed from support/156162-176155/app-client-components-without-dirs-biome to dev/7.4.x December 24, 2025 18:24
});
return (
<div className="text-center staff-credit-content" onClick={contentsClickedHandler}>
<button
Copy link
Contributor

Choose a reason for hiding this comment

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

この要素が button になるの違和感があるが…

biome 的にはそうせざるを得ないのだろうか?

Copy link
Contributor Author

@arafubeatbox arafubeatbox Dec 26, 2025

Choose a reason for hiding this comment

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

GPT-5.2 の回答の引用ですが、

What are the other options instead of button?

• Options if you want the whole area clickable:

  • <button>: best semantics, passes a11y lint; can be fully styled to look like a plain container.
  • <a href="...">: only appropriate if it navigates; still needs a real href.
  • <div role="button" tabIndex={0} onKeyDown=...>: possible, but Biome a11y often still flags it and it’s more work to get keyboard behavior right.
  • <label> tied to a hidden input: hacky, not recommended.

So practically, <button> is the only clean option if the whole area is interactive.

という感じで、結局 div に button のようなインタラクティブ性を持たせるので button にした方が早いよ、という結果になりました。

補足) div のままやろうとすると、
https://biomejs.dev/linter/rules/no-static-element-interactions/
の結果 role="button" を付与するように指定され、さらにその結果、
https://biomejs.dev/linter/rules/use-semantic-elements/
で button にできるよ、と怒られてしまいます。
つまり、非インタラクティブな要素のままにしようとする場合、どれかのルールは ignore しないといけなさそうです。

@yuki-takei yuki-takei changed the base branch from dev/7.4.x to master December 26, 2025 12:03
@yuki-takei yuki-takei merged commit 3d3d3d8 into master Dec 26, 2025
26 checks passed
@yuki-takei yuki-takei deleted the support/156162-176216-app-some-client-components-biome-5 branch December 26, 2025 14:32
@github-actions github-actions bot mentioned this pull request Dec 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants