refactor: フロントエンド開発#81
Merged
Merged
Conversation
Contributor
Walkthroughローカル状態とイベント駆動から、Lit コンテキスト＋Repository パターンへ移行。多数の管理用 UI コンポーネント、リポジトリ層、RepositoryObserver、テーマ／トークン再編を追加し、複数ページをコンテキスト駆動へ書き換えた。 Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/page-admin-account.ts (1)
29-39:⚠️ Potential issue | 🟠 Major
updated()の条件ロジックを確認してください
changedProperties.has('authRepo')はauthRepoオブジェクト参照自体が変更されたかをチェックしますが、RepositoryObserverがrequestUpdate()を呼び出してもauthRepoはchangedPropertiesに含まれません。
accountSuccessの状態変化を検知するには、前回の値を保持して比較するか、別のアプローチが必要です。🔧 修正案: 前回の成功状態を追跡
`@state`() private lastSubmittedAction = '' + + `@state`() + private prevAccountSuccess = '' // ... protected updated(changedProperties: Map<PropertyKey, unknown>) { + const currentSuccess = this.authRepo?.accountSuccess ?? '' if ( - changedProperties.has('authRepo') && - this.authRepo.accountSuccess && + currentSuccess && + currentSuccess !== this.prevAccountSuccess && this.lastSubmittedAction === 'change-email' ) { this.token = '' this.newEmailAddress = '' this.lastSubmittedAction = '' } + this.prevAccountSuccess = currentSuccess }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/page-admin-account.ts` around lines 29 - 39, The updated() logic incorrectly relies on changedProperties.has('authRepo') to detect accountSuccess changes; track the previous accountSuccess value instead and compare it to the current one: add a class field like prevAccountSuccess (initialize in constructor or firstUpdated), then in updated(changedProperties) check if this.authRepo?.accountSuccess is true and this.prevAccountSuccess is false and this.lastSubmittedAction === 'change-email' to clear token/newEmailAddress/lastSubmittedAction, and finally set this.prevAccountSuccess = !!this.authRepo?.accountSuccess so future updates detect changes correctly; reference updated, authRepo, accountSuccess, lastSubmittedAction, and prevAccountSuccess.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/profile/me-profile-certifications-editor.ts`:
- Around line 88-96: The change handler for the me-text-input currently uses
Number(e.detail || '0') which will coerce an empty input into year: 0; update
the handler in the me-text-input `@change` to detect an empty detail (e.detail ===
'' or null/undefined) and instead set year to a sensible fallback (e.g., new
Date().getFullYear()) or to undefined if you want to treat it as absent, by
calling this.updateItem(index, { year: fallbackYear }) or { year: undefined }
accordingly; modify the updateItem call site that expects a numeric year to
accept undefined if you choose that route.
In `@frontend/src/components/admin/profile/me-profile-experiences-editor.ts`:
- Around line 89-97: The change handler for the me-text-input currently forces
empty input to 0 via (e.detail || '0'), causing startYear to be set to year 0;
update the handler in me-profile-experiences-editor.ts (the me-text-input that
calls updateItem) to only convert and set startYear when e.detail is non-empty
(e.g. check e.detail !== '' && e.detail != null) and otherwise pass
null/undefined (or omit the field) so an empty input does not become 0; use
Number(e.detail) or parseInt(e.detail, 10) for the numeric conversion when
present and keep updateItem call and symbol names unchanged.
In `@frontend/src/components/admin/profile/me-profile-skills-editor.ts`:
- Around line 43-48: The splitLines method is duplicated between
me-profile-skills-editor.ts and page-admin-profile.ts; extract it into a shared
exported utility (e.g., export function splitLines(value: string) in a new or
existing utils/stringHelpers module), replace the private splitLines method in
the class in me-profile-skills-editor.ts with an import of the shared
splitLines, and update page-admin-profile.ts to import and use the same exported
function; ensure the utility is properly exported and both files import it and
remove the local duplicated implementations.
- Around line 99-108: The rows attribute on the me-textarea is currently passed
as a string ("4"); change it to pass a numeric value so Lit's Number property
receives a clear number—update the me-textarea usage in me-profile-skills-editor
(the element with .value=${skill.items.join('\n')} and the `@change` handler
calling updateSkill and splitLines) to bind rows as a number (use the property
binding form to pass 4 as a Number) so the me-textarea `@property`({ type: Number
}) receives an actual number rather than a string.
In `@frontend/src/components/admin/ui/me-admin-section.ts`:
- Around line 37-43: The .section rule uses a hardcoded background color
"#ffffff" which breaks theme extensibility; update the .section selector to use
the theme token variable (e.g. replace "#ffffff" with a semantic CSS variable
like var(--color-background) or var(--color-surface)/the existing background
token used across the app) so the section follows theme changes, and ensure the
chosen token exists in your theme variables.
In `@frontend/src/components/admin/ui/me-text-input.ts`:
- Around line 22-32: The _onInput method currently dispatches a
CustomEvent('change') on every keystroke which differs from native semantics
(native change fires on blur); add a concise comment above the _onInput function
explaining this is intentional — that the component emits 'change' for real-time
updates and that callers like page-admin-login expect that behavior — and
optionally note that renaming the event to 'input' would be more conventional if
you later want to align with native semantics.
- Around line 7-8: The value property in MeTextInput is incorrectly typed as
string | number even though HTMLInputElement.value is always a string; change
the component's property declaration for value to string only, update any
related typings/usages in the component (e.g., the `@property`() value declaration
and the _onInput handler/dispatch detail) to reflect string, and add a note in
the component JSDoc or comment that callers should parse to number when using
type="number" (as done in me-profile-skills-editor.ts for sortOrder) so
consumers perform numeric conversion where needed.
In `@frontend/src/components/admin/ui/me-textarea.ts`:
- Around line 30-37: The textarea currently uses property binding
".rows=${this.rows}" which is incorrect because rows is an HTML attribute;
change the binding to an attribute binding "rows=${this.rows}" on the <textarea>
element and ensure this.rows is cast to a string/number if needed; update the
template where ".rows" appears (in the component that defines this.rows and
_onInput) to use "rows=${this.rows}" so the attribute is rendered correctly.
In `@frontend/src/components/app-admin-shell.ts`:
- Around line 22-25: コンストラクタ内で new RepositoryObserver(this, this.authRepo )
を呼んでいますが、@consume で注入される authRepo はコンストラクタ実行時は未定義なので動作しません。RepositoryObserver
の初期化を constructor から取り除き、代わりに connectedCallback() をオーバーライドしてその中で new
RepositoryObserver(this, this.authRepo) を呼び出すようにしてください（page-about.ts
と同様のパターン）。これにより authRepo が注入された後に RepositoryObserver が正しく初期化されます。
In `@frontend/src/components/app-root.ts`:
- Around line 46-52: The constructor currently instantiates
RepositoryObserver(this, this.auth/profile/article) immediately after super();
add a brief clarifying comment in the constructor above those new
RepositoryObserver(...) calls stating that properties decorated with `@provide`
(this.auth, this.profile, this.article) are initialized as class fields after
super() so the observers are safe to create here; mention the `@provide` decorator
and the RepositoryObserver constructor to make the intent and ordering explicit
for future readers.
In `@frontend/src/domain/ArticleRepository.ts`:
- Around line 18-21: The setter setAdminDirty currently assigns this._adminDirty
and calls notifyChange() unconditionally; change it to compare the incoming
dirty with the current this._adminDirty and only update this._adminDirty and
call notifyChange() when the value actually differs to avoid unnecessary
notifications and renders (use the existing setAdminDirty method and
this._adminDirty field to implement the check).
In `@frontend/src/domain/AuthRepository.ts`:
- Around line 96-106: The logout handler currently sets the same message into
both _loginNotice and _accountSuccess after await logout(), causing duplicate UI
notifications; update the logic in the logout method (where logout() is called)
to set only one of these (e.g., keep _loginNotice = 'ログアウトしました。' and remove or
clear _accountSuccess) or change _accountSuccess to an appropriate distinct
success state, then ensure notifyChange() behavior remains the same so the UI is
updated once.
In `@frontend/src/domain/ProfileRepository.ts`:
- Around line 70-82: The loadPublicProfile method currently swallows errors in
its catch block; update it to surface failures by either logging the error
(e.g., console.warn with the caught error) or by setting a new state field
(e.g., this.publicError) so consumers can react; specifically modify
loadPublicProfile to catch (err) and then call console.warn("loadPublicProfile
failed", err) or assign this.publicError = err before finally clearing
_publicLoading and calling notifyChange, referencing the existing symbols
loadPublicProfile, _publicProfile, _publicLoading, _internalFetch, and
notifyChange.
- Around line 101-112: _internaFetch currently assigns the same fetched object p
to both _publicProfile and _adminProfile causing shared-reference mutations;
change it so each gets its own copy (e.g., use cloneMeProfile or a shallow/deep
clone) after awaiting getMe() and before assigning to _publicProfile and
_adminProfile so mutations to one profile don't affect the other; ensure you
import cloneMeProfile if you use it and update assignments in _internalFetch
accordingly.
In `@frontend/src/pages/page-about.ts`:
- Around line 16-19: The constructor currently creates a new
RepositoryObserver(this, this.profileRepo) while the `@consume-injected` property
profileRepo is undefined until the element is connected, causing hostConnected()
to call addEventListener on undefined; move the RepositoryObserver creation into
the element's connectedCallback (or hostConnected) so it runs after the element
is connected and profileRepo is available, e.g., instantiate RepositoryObserver
in connectedCallback and add a defensive check (if (this.profileRepo) ...) to
avoid creating it when the injected value is still missing; apply the same
change pattern for other files with the same pattern (page-top.ts,
page-admin-profile.ts, page-admin-articles.ts, page-admin-login.ts,
app-admin-shell.ts, app-root.ts) referencing their constructors and
connectedCallback/hostConnected methods.
In `@frontend/src/pages/page-admin-account.ts`:
- Around line 24-27: The constructor creates a new RepositoryObserver using
authRepo before the `@consume-injected` property is available; move the
RepositoryObserver initialization out of constructor() into connectedCallback()
so authRepo is defined when used, i.e., remove new RepositoryObserver(this,
this.authRepo) from constructor() and instead instantiate RepositoryObserver
inside connectedCallback() (or lazily on first connection) referencing
this.authRepo and this to locate the correct code paths.
In `@frontend/src/pages/page-admin-articles.ts`:
- Around line 85-88: The constructor currently instantiates RepositoryObserver
with this.articleRepo before the `@consume-injected` articleRepo is available;
move the initialization into a lifecycle hook (e.g., connectedCallback or
firstUpdated) so RepositoryObserver is created after articleRepo is injected.
Locate the class constructor and replace the new RepositoryObserver(this,
this.articleRepo) call by creating the observer inside connectedCallback() or
firstUpdated(), referencing the same RepositoryObserver and articleRepo symbols,
and ensure any existing cleanup or duplicate-instantiation checks are preserved.
In `@frontend/src/pages/page-admin-profile.ts`:
- Around line 32-35: The constructor creates a new RepositoryObserver with
this.profileRepo before the `@consume` property is guaranteed initialized; move
the RepositoryObserver initialization out of the constructor into
connectedCallback so that RepositoryObserver is created after the
`@consume-injected` profileRepo is available (e.g., remove new
RepositoryObserver(this, this.profileRepo) from the constructor and invoke it
inside connectedCallback where this.profileRepo is stable).
In `@frontend/src/pages/page-top.ts`:
- Around line 28-31: The RepositoryObserver is being constructed in the class
constructor while the `@consume-injected` property this.profileRepo may still be
undefined; move the new RepositoryObserver(this, this.profileRepo) creation into
a lifecycle method (e.g., connectedCallback() or firstUpdated()) so the consumer
injection is complete before use. Locate the constructor that currently calls
new RepositoryObserver(...) and remove that call, then add the observer
initialization in connectedCallback() or firstUpdated(), checking that
this.profileRepo is defined before instantiating RepositoryObserver.
---
Outside diff comments:
In `@frontend/src/pages/page-admin-account.ts`:
- Around line 29-39: The updated() logic incorrectly relies on
changedProperties.has('authRepo') to detect accountSuccess changes; track the
previous accountSuccess value instead and compare it to the current one: add a
class field like prevAccountSuccess (initialize in constructor or firstUpdated),
then in updated(changedProperties) check if this.authRepo?.accountSuccess is
true and this.prevAccountSuccess is false and this.lastSubmittedAction ===
'change-email' to clear token/newEmailAddress/lastSubmittedAction, and finally
set this.prevAccountSuccess = !!this.authRepo?.accountSuccess so future updates
detect changes correctly; reference updated, authRepo, accountSuccess,
lastSubmittedAction, and prevAccountSuccess.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 329e9281-c976-41b8-bc3b-9c0e1c1c38b6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
frontend/package.jsonfrontend/src/components/admin/profile/me-profile-certifications-editor.tsfrontend/src/components/admin/profile/me-profile-experiences-editor.tsfrontend/src/components/admin/profile/me-profile-links-editor.tsfrontend/src/components/admin/profile/me-profile-skills-editor.tsfrontend/src/components/admin/ui/me-admin-field.tsfrontend/src/components/admin/ui/me-admin-panel.tsfrontend/src/components/admin/ui/me-admin-section.tsfrontend/src/components/admin/ui/me-select.tsfrontend/src/components/admin/ui/me-text-input.tsfrontend/src/components/admin/ui/me-textarea.tsfrontend/src/components/app-admin-shell.tsfrontend/src/components/app-root.tsfrontend/src/contexts/article-context.tsfrontend/src/contexts/auth-context.tsfrontend/src/contexts/profile-context.tsfrontend/src/controllers/RepositoryObserver.tsfrontend/src/domain/ArticleRepository.tsfrontend/src/domain/AuthRepository.tsfrontend/src/domain/ProfileRepository.tsfrontend/src/domain/Repository.tsfrontend/src/index.cssfrontend/src/pages/page-about.tsfrontend/src/pages/page-admin-account.tsfrontend/src/pages/page-admin-articles.tsfrontend/src/pages/page-admin-login.tsfrontend/src/pages/page-admin-profile.tsfrontend/src/pages/page-top.tsfrontend/src/styles/theme-admin.cssfrontend/src/styles/theme-public.cssfrontend/src/styles/tokens.cssfrontend/src/tokens.css
💤 Files with no reviewable changes (1)
- frontend/src/tokens.css
Comment on lines
+88
to
+96
| <me-text-input | ||
| label="Year" | ||
| type="number" | ||
| .value=${String(cert.year)} | ||
| @change=${(e: CustomEvent) => | ||
| this.updateItem(index, { | ||
| year: Number(e.detail || '0'), | ||
| })} | ||
| ></me-text-input> |
Contributor
There was a problem hiding this comment.
年が空の場合に0が設定される可能性があります。
e.detail || '0' により、ユーザーが年を空にした場合に year: 0 が設定されます。これは意図した動作でしょうか？バリデーションエラーとして扱うか、undefined を許容する方が適切かもしれません。
🛡️ 修正案: 空の場合は現在年をフォールバックとして使用
<me-text-input
label="Year"
type="number"
.value=${String(cert.year)}
`@change`=${(e: CustomEvent) =>
this.updateItem(index, {
- year: Number(e.detail || '0'),
+ year: e.detail ? Number(e.detail) : new Date().getFullYear(),
})}
></me-text-input>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <me-text-input | |
| label="Year" | |
| type="number" | |
| .value=${String(cert.year)} | |
| @change=${(e: CustomEvent) => | |
| this.updateItem(index, { | |
| year: Number(e.detail || '0'), | |
| })} | |
| ></me-text-input> | |
| <me-text-input | |
| label="Year" | |
| type="number" | |
| .value=${String(cert.year)} | |
| `@change`=${(e: CustomEvent) => | |
| this.updateItem(index, { | |
| year: e.detail ? Number(e.detail) : new Date().getFullYear(), | |
| })} | |
| ></me-text-input> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/profile/me-profile-certifications-editor.ts`
around lines 88 - 96, The change handler for the me-text-input currently uses
Number(e.detail || '0') which will coerce an empty input into year: 0; update
the handler in the me-text-input `@change` to detect an empty detail (e.detail ===
'' or null/undefined) and instead set year to a sensible fallback (e.g., new
Date().getFullYear()) or to undefined if you want to treat it as absent, by
calling this.updateItem(index, { year: fallbackYear }) or { year: undefined }
accordingly; modify the updateItem call site that expects a numeric year to
accept undefined if you choose that route.
Comment on lines
+89
to
+97
| <me-text-input | ||
| label="Start year" | ||
| type="number" | ||
| .value=${String(exp.startYear)} | ||
| @change=${(e: CustomEvent) => | ||
| this.updateItem(index, { | ||
| startYear: Number(e.detail || '0'), | ||
| })} | ||
| ></me-text-input> |
Contributor
There was a problem hiding this comment.
startYearが空の場合に0が設定されます。
certifications-editor と同様に、e.detail || '0' により空の入力が startYear: 0 になります。これは西暦0年として扱われる可能性があり、意図しない動作かもしれません。
🛡️ 修正案
<me-text-input
label="Start year"
type="number"
.value=${String(exp.startYear)}
`@change`=${(e: CustomEvent) =>
this.updateItem(index, {
- startYear: Number(e.detail || '0'),
+ startYear: e.detail ? Number(e.detail) : new Date().getFullYear(),
})}
></me-text-input>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/profile/me-profile-experiences-editor.ts`
around lines 89 - 97, The change handler for the me-text-input currently forces
empty input to 0 via (e.detail || '0'), causing startYear to be set to year 0;
update the handler in me-profile-experiences-editor.ts (the me-text-input that
calls updateItem) to only convert and set startYear when e.detail is non-empty
(e.g. check e.detail !== '' && e.detail != null) and otherwise pass
null/undefined (or omit the field) so an empty input does not become 0; use
Number(e.detail) or parseInt(e.detail, 10) for the numeric conversion when
present and keep updateItem call and symbol names unchanged.
Comment on lines
+43
to
+48
| private splitLines(value: string) { | ||
| return value | ||
| .split('\n') | ||
| .map((item) => item.trim()) | ||
| .filter(Boolean) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
splitLines ユーティリティの重複
この splitLines メソッドは page-admin-profile.ts にも同様の実装があります。将来的には共有ユーティリティとして抽出することを検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/profile/me-profile-skills-editor.ts` around
lines 43 - 48, The splitLines method is duplicated between
me-profile-skills-editor.ts and page-admin-profile.ts; extract it into a shared
exported utility (e.g., export function splitLines(value: string) in a new or
existing utils/stringHelpers module), replace the private splitLines method in
the class in me-profile-skills-editor.ts with an import of the shared
splitLines, and update page-admin-profile.ts to import and use the same exported
function; ensure the utility is properly exported and both files import it and
remove the local duplicated implementations.
Comment on lines
+99
to
+108
| <me-textarea | ||
| label="Items（改行区切り）" | ||
| .value=${skill.items.join('\n')} | ||
| rows="4" | ||
| class="field-wide" | ||
| @change=${(e: CustomEvent) => | ||
| this.updateSkill(index, { | ||
| items: this.splitLines(e.detail), | ||
| })} | ||
| ></me-textarea> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
rows 属性の型を確認してください
me-textarea の rows プロパティは @property({ type: Number }) で定義されています。rows="4" は文字列として渡されますが、Lit の type: Number コンバーターにより自動変換されるため動作しますが、明示的に数値を渡す方が意図が明確です。
♻️ より明示的な記述
<me-textarea
label="Items（改行区切り）"
.value=${skill.items.join('\n')}
- rows="4"
+ .rows=${4}
class="field-wide"
`@change`=${(e: CustomEvent) =>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <me-textarea | |
| label="Items（改行区切り）" | |
| .value=${skill.items.join('\n')} | |
| rows="4" | |
| class="field-wide" | |
| @change=${(e: CustomEvent) => | |
| this.updateSkill(index, { | |
| items: this.splitLines(e.detail), | |
| })} | |
| ></me-textarea> | |
| <me-textarea | |
| label="Items（改行区切り）" | |
| .value=${skill.items.join('\n')} | |
| .rows=${4} | |
| class="field-wide" | |
| `@change`=${(e: CustomEvent) => | |
| this.updateSkill(index, { | |
| items: this.splitLines(e.detail), | |
| })} | |
| ></me-textarea> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/profile/me-profile-skills-editor.ts` around
lines 99 - 108, The rows attribute on the me-textarea is currently passed as a
string ("4"); change it to pass a numeric value so Lit's Number property
receives a clear number—update the me-textarea usage in me-profile-skills-editor
(the element with .value=${skill.items.join('\n')} and the `@change` handler
calling updateSkill and splitLines) to bind rows as a number (use the property
binding form to pass 4 as a Number) so the me-textarea `@property`({ type: Number
}) receives an actual number rather than a string.
- Repository (Domain) - Context (Infrastructure) - RepositoryObserver (Adapter) - Atomic UI Components (View Primitives) - Domain Editors (View Composition) - Page Orchestrators (View Controller)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/pages/page-top.ts (1)
98-103:⚠️ Potential issue | 🔴 Critical保存済みプロフィール URL を未検証のまま
hrefに入れないでください。ここは公開ページなので、
link.urlにjavascript:のような危険なスキームが保存されると、クリック時にそのまま実行されます。描画前か保存前にhttp:/https:/mailto:などへ明示的に絞るべきです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/page-top.ts` around lines 98 - 103, The code directly inserts user-supplied link.url into the anchor href inside the p.links.map render, which allows dangerous schemes like javascript:; add a validation/sanitization step (e.g., implement an isSafeUrl or sanitizeUrl helper and call it from the rendering code used in page-top) that only permits explicit schemes such as http, https, mailto (and optionally tel), and if the URL is unsafe fall back to a safe placeholder (e.g., '#' or omitted href) and log or skip the link; update the p.links.map usage to use the sanitized/validated URL instead of link.url.frontend/src/pages/page-admin-account.ts (1)
31-40:⚠️ Potential issue | 🟠 Major
updated()の条件では、メール変更成功後に入力欄がクリアされません。
authRepoは@consume({ context: authContext, subscribe: true })で setter/getter ペアに注入されており、@property()や@state()として宣言されていません。そのため、authRepoの状態が変化してもコンポーネントの再描画時にchangedProperties.has('authRepo')は真になりません。結果として、changeEmail()成功後もtokenとnewEmailAddressはクリアされずフォーム内に残ります。メール変更成功時のクリアは、
handleChangeEmail()のawait this.authRepo.changeEmail(...)直後に実行するのが確実です：private async handleChangeEmail(event: Event) { event.preventDefault() this.lastSubmittedAction = 'change-email' await this.authRepo.changeEmail({ token: this.token.trim(), newEmailAddress: this.newEmailAddress.trim(), }) // クリアロジックをここに移動 this.token = '' this.newEmailAddress = '' this.lastSubmittedAction = '' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/page-admin-account.ts` around lines 31 - 40, Move the form-clear logic out of updated() into the success path immediately after awaiting the authRepo changeEmail call: in handleChangeEmail(), after await this.authRepo.changeEmail({...}), reset this.token, this.newEmailAddress and this.lastSubmittedAction to empty values; remove or keep the updated() branch that checks changedProperties.has('authRepo') as no-op since authRepo is injected (refer to updated(), authRepo, handleChangeEmail(), changeEmail(), token, newEmailAddress, lastSubmittedAction).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/ui/me-text-input.ts`:
- Around line 5-27: The host element MeTextInput needs a public focus() method
that delegates focus to its inner <input>; add a focus() method on the
MeTextInput class that queries the shadow DOM for the internal input element
(e.g., this.shadowRoot!.querySelector('input')) and calls its focus() (and
optionally set selectionStart/selectionEnd or call setSelectionRange if needed
for caret placement); ensure the method is public so external callers (like
page-admin-login.ts calling focus()) actually focus the inner input rather than
the host element.
- Around line 31-43: The label element isn't associated with the input; add a
stable id and wire label's for to it so clicks and assistive tech focus the
input. Introduce a unique id field (e.g., this._inputId) on the component
(generate in the constructor or connectedCallback, or derive from an existing
name property) and set the input's id to that value and the label's for
attribute to the same id in the template where label and input are rendered;
keep existing properties (.value, `@input`=_onInput, etc.) unchanged. Use the
component/class name (MeTextInput) and template elements (label, input,
_onInput) to locate where to add the id/for wiring.
- Around line 33-43: me-text-input must be converted to a form-associated custom
element so the inner <input> participates in parent form validation: add static
formAssociated = true on the me-text-input class, call this._internals =
this.attachInternals() (e.g., in the constructor), and update the component to
report state via this._internals.setFormValue(...) whenever the internal input
value changes (update _onInput to call setFormValue with the current value and
sync required/disabled/name attributes), and use
this._internals.setValidity(...) to reflect validity; also implement
formDisabledCallback and formResetCallback on me-text-input to sync disabled
state and reset the internal input.
In `@frontend/src/components/app-admin-shell.ts`:
- Around line 13-21: The component currently uses a definite assignment
assertion on private _authRepo and then unconditionally reads
this.authRepo.status in render(), which throws if the context provider hasn't
supplied a repo; change private _authRepo!: IAuthRepository to an optional field
(private _authRepo?: IAuthRepository) and update any reads in render() (and
elsewhere) to guard via optional chaining (e.g., this.authRepo?.status ===
'authenticated') or null-checks; ensure the authRepo getter/setter remain
consistent with the optional type and that RepositoryObserver is only created
when repo is truthy.
- Around line 13-17: authRepo セッターで毎回 new RepositoryObserver(...) を作らないでください —
現状の実装は host.addController() → hostConnected() でリスナーが登録され、古い controller が削除されないため
listener が蓄積し requestUpdate() が重複発火します。修正方法：コンポーネントに単一の observer インスタンス（例:
this._repoObserver）を保持し、set authRepo(repo: IAuthRepository) 内では新規生成せず既存の
RepositoryObserver に監視対象を差し替えるか（または自前で listener を登録/解除して管理する）、古い controller
を明示的に削除してから host.addController() するように変更してください（参照箇所: RepositoryObserver,
host.addController, hostConnected, requestUpdate）。また render 側で
this.authRepo.status を直接参照しないで、_authRepo が存在することをチェックしてからアクセスするように修正してください（参照:
_authRepo, this.authRepo.status）。
In `@frontend/src/pages/page-about.ts`:
- Around line 11-15: profileRepo セッターが毎回 new RepositoryObserver(this, repo)
を呼んで多重登録を招いているので、RepositoryObserver をインスタンス変数（例:
this._repoObserver）として保持し、既に存在する observer がなければ生成、既存と別の repo に差し替える場合は古い
observer を破棄してから新規生成するように修正してください。対象は profileRepo セッター、_profileRepo フィールドと
RepositoryObserver の生成箇所で、observer のライフサイクル（作成・再利用・破棄）を明示的に扱う実装にしてください。
In `@frontend/src/pages/page-admin-account.ts`:
- Around line 12-16: authRepo の setter が再注入ごとに新しい RepositoryObserver
を作り続けているため重複通知が発生します; 修正は setter 内で同一 repo の再設定を無視するか既存の observer
を破棄してから新規作成すること（例: check this._authRepo === repo and return early, or keep a
this._repoObserver property and call dispose/remove on it before `new
RepositoryObserver(this, repo)`), updating references to this._authRepo and
RepositoryObserver accordingly.
In `@frontend/src/pages/page-admin-articles.ts`:
- Around line 49-53: The setter for articleRepo creates a new RepositoryObserver
every time, causing duplicate subscriptions; store a single observer instance
(e.g. add a private field _repoObserver) and in the articleRepo setter either
reuse it or if replacing the repo first call the observer's teardown method
(dispose()/unsubscribe()/disconnect() as provided) to remove the old
subscription before creating a new RepositoryObserver(this, repo); update
references from creating unconditional new RepositoryObserver to using
_repoObserver so only one observer exists at a time.
In `@frontend/src/pages/page-admin-login.ts`:
- Around line 12-16: authRepo セッターで毎回 new RepositoryObserver
を作っているため同じリポジトリに対して多重登録が発生しています; this._authRepo に加え現在のオブザーバーを保持するプロパティ（例:
this._authRepoObserver）を追加し、セッター実行時に既存のオブザーバーがあれば解除/破棄してから新しい RepositoryObserver
を作る、あるいは既に同じ repo を監視していれば再作成しないようにして多重登録を防いでください（対象シンボル: authRepo セッター,
this._authRepo, RepositoryObserver, this._authRepoObserver）。
In `@frontend/src/pages/page-admin-profile.ts`:
- Around line 25-29: setter のたびに新しい RepositoryObserver
を生成して購読が重複しているので、profileRepo セッター（set profileRepo(repo: IProfileRepository)）で同一
repo の再代入を早期 return し、既存の observer を this._repoObserver のようなプロパティで保持している場合は新しい
repo に差し替える前に既存 observer を解除／破棄してから（古い repo の購読を止めて）新しい RepositoryObserver
を作るように修正してください。具体的には set profileRepo 内で repo === this._profileRepo
をチェックして何もしない、差し替え時は既存 this._repoObserver を解約（dispose/unsubscribe メソッド呼び出し）してから
this._profileRepo = repo と this._repoObserver = new RepositoryObserver(this,
repo) を行う流れにしてください。
In `@frontend/src/pages/page-top.ts`:
- Around line 14-18: The RepositoryObserver instances are being accumulated each
time the profileRepo setter runs; change the setter (profileRepo) to manage a
single observer instance by storing a field (e.g., this._profileObserver), first
checking if the incoming repo is identical to this._profileRepo and returning
early, and if replacing the repo, call a teardown/dispose/unsubscribe on the
existing this._profileObserver before creating a new RepositoryObserver(this,
repo); update this._profileRepo only after handling the observer swap so
duplicate observers are not left attached.
---
Outside diff comments:
In `@frontend/src/pages/page-admin-account.ts`:
- Around line 31-40: Move the form-clear logic out of updated() into the success
path immediately after awaiting the authRepo changeEmail call: in
handleChangeEmail(), after await this.authRepo.changeEmail({...}), reset
this.token, this.newEmailAddress and this.lastSubmittedAction to empty values;
remove or keep the updated() branch that checks
changedProperties.has('authRepo') as no-op since authRepo is injected (refer to
updated(), authRepo, handleChangeEmail(), changeEmail(), token, newEmailAddress,
lastSubmittedAction).
In `@frontend/src/pages/page-top.ts`:
- Around line 98-103: The code directly inserts user-supplied link.url into the
anchor href inside the p.links.map render, which allows dangerous schemes like
javascript:; add a validation/sanitization step (e.g., implement an isSafeUrl or
sanitizeUrl helper and call it from the rendering code used in page-top) that
only permits explicit schemes such as http, https, mailto (and optionally tel),
and if the URL is unsafe fall back to a safe placeholder (e.g., '#' or omitted
href) and log or skip the link; update the p.links.map usage to use the
sanitized/validated URL instead of link.url.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9ccc36e-48e3-422e-a3a8-a0f532ccdfb1
📒 Files selected for processing (9)
frontend/src/components/admin/ui/me-text-input.tsfrontend/src/components/app-admin-shell.tsfrontend/src/pages/page-about.tsfrontend/src/pages/page-admin-account.tsfrontend/src/pages/page-admin-articles.tsfrontend/src/pages/page-admin-login.tsfrontend/src/pages/page-admin-profile.tsfrontend/src/pages/page-top.tsfrontend/src/styles/theme-admin.css
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
frontend/src/components/admin/ui/me-text-input.ts (3)
7-10:⚠️ Potential issue | 🟡 Minor
valueの型はstringに揃えてください。
HTMLInputElement.valueも Line 29 のdetailも常に文字列です。string | numberのままだとtype="number"で数値が来るように見えて、利用側の型解釈を誤らせます。修正案
- `@property`() value: string | number = '' + `@property`() value = ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/ui/me-text-input.ts` around lines 7 - 10, The property declaration for the component's value should be changed to a plain string to match HTMLInputElement.value and the event detail; update the `@property`() value declaration in me-text-input (the value property) from "string | number" to "string" and ensure any handlers or consumers (e.g., input event handlers that read event.detail or assign to value) treat it as a string so downstream type assumptions align with actual runtime values.
39-50:⚠️ Potential issue | 🟠 Major
labelと内部<input>が未関連付けです。Line 39 の
labelにforがないため、ラベルクリックで入力欄へフォーカスできません。id/forを追加してください。修正案
export class MeTextInput extends LitElement { + private readonly _inputId = `me-text-input-${crypto.randomUUID()}` + `@property`() label = '' `@property`() name = '' @@ <div class=${classMap({ field: true, disabled: this.disabled })}> - ${this.label ? html`<label class="label">${this.label}</label>` : null} + ${this.label + ? html`<label class="label" for=${this._inputId}>${this.label}</label>` + : null} <input + id=${this._inputId} .type=${this.type} .name=${this.name} .autocomplete=${this.autocomplete}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/ui/me-text-input.ts` around lines 39 - 50, The label in the template is not associated with the input; add a matching id on the input and a for attribute on the label so clicking the label focuses the input. Update the template where the label and input are rendered (the label element and the input element in the render template) to use the same id value (preferably a component-level property like this.id or a generated fallback such as this._inputId), and if you add a generated id implement it in the constructor/connectedCallback so it is stable for the component lifecycle; ensure the label uses for=${this.idOrFallback} and the input uses id=${this.idOrFallback} (keeping existing bindings like .name, .value unchanged).
40-49:⚠️ Potential issue | 🟠 Major
required/nameが親フォームに効いていません。内部
<input>が Shadow DOM 内にあるだけでは、祖先<form>のネイティブ検証・送信対象になりません。page-admin-profile.tsとpage-admin-articles.tsで付けているrequiredが実質無効になるので、formAssociatedとElementInternalsでホスト側を form-associated custom element にしてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/ui/me-text-input.ts` around lines 40 - 49, Make the custom element form-associated so host-level name/required/disabled participate in ancestor <form>: add static get formAssociated() { return true } to the MeTextInput class and create an ElementInternals instance (e.g. this._internals = this.attachInternals()) during construction/connectedCallback; whenever the internal <input> value or disabled state changes call this._internals.setFormValue(this.disabled ? null : this.value) and update validity with this._internals.setValidity(...) as appropriate; ensure the host reflects/observes the name/required/disabled/readonly attributes (mirror attribute changes to the internal input and call setFormValue when value changes) and implement form callbacks (formDisabledCallback/formResetCallback) to forward form reset/disable behavior to the internal input so parent forms see the required/name semantics.frontend/src/pages/page-admin-articles.ts (1)
49-53:⚠️ Potential issue | 🟠 Major
RepositoryObserverを setter のたびに増やさないでください。Line 50-52 は context の再配信や repo 差し替えのたびに新しい observer を追加するので、同じ
changeで複数回requestUpdate()されます。observer を 1 つだけ保持し、同一 repo の再代入は early return、差し替え時だけ購読を張り替えてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/page-admin-articles.ts` around lines 49 - 53, The setter for articleRepo is creating a new RepositoryObserver on every assignment which leads to duplicate observers and repeated requestUpdate() calls; change articleRepo (the setter) to ignore reassignments of the same repo (early return when repo === this._articleRepo), keep a single RepositoryObserver instance on the component (e.g. this._repoObserver), and on repo replacement unsubscribe/destroy the previous observer and create a new one bound to the new repo; update this._articleRepo only after switching observers so you never accumulate multiple RepositoryObserver instances.frontend/src/pages/page-admin-profile.ts (1)
25-29:⚠️ Potential issue | 🟠 Major
RepositoryObserverを setter のたびに増やさないでください。この setter は再実行され得ますが、毎回
new RepositoryObserver(this, repo)すると購読が積み増されます。observer を 1 つだけ保持して、同一 repo の再代入は何もしない、repo 差し替え時だけ古い購読を外してから張り替える形にしてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/page-admin-profile.ts` around lines 25 - 29, setter のたびに new RepositoryObserver(...) して購読が重複しているので、profileRepo セッターで this._profileObserver（新しい private フィールド）に単一のオブザーバを保持し、もし repo が同一なら何もしない、repo が差し替わる場合は既存の this._profileObserver を解除する（dispose/unsubscribe/close などの適切なメソッドを呼ぶ）してから新しい RepositoryObserver(this, repo) を代入するように変更してください（参照箇所: setter profileRepo, フィールド this._profileRepo, クラス RepositoryObserver）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/ui/me-textarea.ts`:
- Around line 29-37: The label element is not associated with the textarea, so
add a stable id and wire it: introduce a component property (e.g., inputId or
_textareaId) that is assigned once (constructor or connectedCallback) so it
remains stable across renders, set the textarea id to that property and set the
label's for attribute to the same value (update the template where label and
textarea are rendered). Ensure the id is stable (not regenerated each render)
and continue to bind existing properties like this.rows, .value, .placeholder
and the `@input` handler (_onInput).
---
Duplicate comments:
In `@frontend/src/components/admin/ui/me-text-input.ts`:
- Around line 7-10: The property declaration for the component's value should be
changed to a plain string to match HTMLInputElement.value and the event detail;
update the `@property`() value declaration in me-text-input (the value property)
from "string | number" to "string" and ensure any handlers or consumers (e.g.,
input event handlers that read event.detail or assign to value) treat it as a
string so downstream type assumptions align with actual runtime values.
- Around line 39-50: The label in the template is not associated with the input;
add a matching id on the input and a for attribute on the label so clicking the
label focuses the input. Update the template where the label and input are
rendered (the label element and the input element in the render template) to use
the same id value (preferably a component-level property like this.id or a
generated fallback such as this._inputId), and if you add a generated id
implement it in the constructor/connectedCallback so it is stable for the
component lifecycle; ensure the label uses for=${this.idOrFallback} and the
input uses id=${this.idOrFallback} (keeping existing bindings like .name, .value
unchanged).
- Around line 40-49: Make the custom element form-associated so host-level
name/required/disabled participate in ancestor <form>: add static get
formAssociated() { return true } to the MeTextInput class and create an
ElementInternals instance (e.g. this._internals = this.attachInternals()) during
construction/connectedCallback; whenever the internal <input> value or disabled
state changes call this._internals.setFormValue(this.disabled ? null :
this.value) and update validity with this._internals.setValidity(...) as
appropriate; ensure the host reflects/observes the
name/required/disabled/readonly attributes (mirror attribute changes to the
internal input and call setFormValue when value changes) and implement form
callbacks (formDisabledCallback/formResetCallback) to forward form reset/disable
behavior to the internal input so parent forms see the required/name semantics.
In `@frontend/src/pages/page-admin-articles.ts`:
- Around line 49-53: The setter for articleRepo is creating a new
RepositoryObserver on every assignment which leads to duplicate observers and
repeated requestUpdate() calls; change articleRepo (the setter) to ignore
reassignments of the same repo (early return when repo === this._articleRepo),
keep a single RepositoryObserver instance on the component (e.g.
this._repoObserver), and on repo replacement unsubscribe/destroy the previous
observer and create a new one bound to the new repo; update this._articleRepo
only after switching observers so you never accumulate multiple
RepositoryObserver instances.
In `@frontend/src/pages/page-admin-profile.ts`:
- Around line 25-29: setter のたびに new RepositoryObserver(...)
して購読が重複しているので、profileRepo セッターで this._profileObserver（新しい private
フィールド）に単一のオブザーバを保持し、もし repo が同一なら何もしない、repo が差し替わる場合は既存の this._profileObserver
を解除する（dispose/unsubscribe/close などの適切なメソッドを呼ぶ）してから新しい RepositoryObserver(this,
repo) を代入するように変更してください（参照箇所: setter profileRepo, フィールド this._profileRepo, クラス
RepositoryObserver）。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 265e74e9-6642-4d0f-b106-e63f210ecd94
📒 Files selected for processing (5)
frontend/src/components/admin/ui/me-admin-section.tsfrontend/src/components/admin/ui/me-text-input.tsfrontend/src/components/admin/ui/me-textarea.tsfrontend/src/pages/page-admin-articles.tsfrontend/src/pages/page-admin-profile.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
frontend/src/components/admin/ui/me-text-input.ts (1)
21-24:⚠️ Potential issue | 🟠 Major
required/type="email"の検証が form に伝わっていません。
setFormValue()はされていますが、host 側の validity を更新していないのでme-text-inputは常に「有効」扱いです。frontend/src/pages/page-admin-account.tsLine 92-149 は submit 時にそのままFormDataを読むため、空の token や不正な email でも送れてしまいます。内部<input>のvalidity/validationMessageをElementInternals.setValidity()に同期してください。For a form-associated custom element using ElementInternals, does putting required/type="email" on an inner shadow-DOM <input> automatically participate in ancestor form validation, or must the host mirror validity via ElementInternals.setValidity()?Also applies to: 48-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/ui/me-text-input.ts` around lines 21 - 24, The host element me-text-input currently calls setFormValue() but doesn't mirror the internal <input>'s validity into its ElementInternals, so required/type="email" never blocks form submission; update the component to call ElementInternals.setValidity() with the internal input's validity state and validationMessage whenever the internal <input> changes (input/change/blur) and inside setFormValue(), so invalid email or empty required token sets host validity and prevents submit. Use the existing ElementInternals instance (from attachInternals()) and read the internal input.validity and input.validationMessage to construct the setValidity() call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/profile/me-profile-links-editor.ts`:
- Around line 42-55: The links editor currently allows empty link items to be
added and propagated because addItem() creates {platform:'', url:''} and
dispatchChange(...) serializes them; fix this by validating/sanitizing link
items before propagating or saving: update addItem/removeItem/updateItem or the
shared dispatchChange/submit path to trim fields and either prevent adding empty
items or filter out entries where platform or url are empty (check
MeLink.platform and MeLink.url), and ensure the save handler in
page-admin-profile.ts validates/filters links again before persisting so no
incomplete link objects reach storage.
In `@frontend/src/components/admin/ui/me-select.ts`:
- Around line 23-26: The formResetCallback currently forces this.value and
this._internals.setFormValue to empty string; change it to derive the reset
value from the component's options so form state and visual selection stay
consistent: in formResetCallback() get the first option's value (e.g., from
this.options or the rendered <option> list) and use that value if present,
otherwise fall back to '' and then set both this.value and
this._internals.setFormValue(resetValue); ensure you reference the existing
formResetCallback method and the component's options collection (e.g.,
this.options or whatever property holds option elements) when implementing the
change.
- Around line 61-70: ネイティブ <select> 内に <slot> を置くとブラウザで機能しないので、テンプレートの <select
id=${this._inputId} ...> から <slot> を削除し、代わりにスロット要素（例: クラス/プロパティにある slot 要素）で
'slotchange' を監視して slot.assignedElements() から返された <option> 要素を実際の <select>（識別子:
this._inputId / 要素参照）へ移動/appendChild する処理を実装してください；既存のハンドラ/プロパティ（_onChange,
name, value, disabled, required）はそのまま維持して select に適用してください。
In `@frontend/src/components/admin/ui/me-textarea.ts`:
- Around line 13-15: 内部 textarea の検証状態がホストに伝播されておらず親フォームに反映されないため、_onInput() と
updated() 内で内部 textarea の validity と validationMessage を
ElementInternals.setValidity() に反映するよう実装してください: コンポーネントの ElementInternals
を取得（this.attachInternals() を既に使っていなければ初期化）し、内部 <textarea> をアンカー要素として扱いその
validity（valid, validity object）と validationMessage を基に setValidity() を呼び出して
host の状態を更新し、また setFormValue() 呼び出し後に必ず同期するよう修正してください（参照シンボル: required
プロパティ、setFormValue(), _onInput(), updated(), ElementInternals.setValidity(),
textarea をアンカー要素として指定）。
In `@frontend/src/controllers/RepositoryObserver.ts`:
- Around line 11-37: RepositoryObserver currently only removes the repository
event listener in disconnect(), leaving the controller registered on the host so
stale controllers get reconnected; add a public dispose/destroy method on
RepositoryObserver that calls this.disconnect() and also removes the controller
from the host (call host.removeController(this) or the equivalent on
ReactiveControllerHost), and update replacement sites to call that dispose
method when swapping observers; reference the class and its methods
(constructor, hostConnected, hostDisconnected, connect, disconnect, add a new
dispose/destroy) so you remove the stale controller from the host as part of
cleanup.
In `@frontend/src/pages/page-admin-articles.ts`:
- Around line 629-631: After successfully saving the draft (after
this.setBaseline(cloneArticleDraft(draft)) and awaiting Promise.all([...]) ),
also clear the repository-level dirty flag by calling
articleRepo.setAdminDirty(false) so the global dirty indicator updates; locate
the save flow around setBaseline/cloneArticleDraft/reloadArticles/refreshTags
and add articleRepo.setAdminDirty(false) immediately after setting
this.localDirty = false (or instead of) to ensure the adminDirty state is reset
like ProfileRepository.saveAdminProfile does.
- Around line 49-60: The field _articleRepo currently uses a definite assignment
assertion which can hide runtime errors; change private _articleRepo!:
IArticleRepository to an optional field (private _articleRepo?:
IArticleRepository) and keep the articleRepo setter/getter logic but accept
undefined; update all usages of articleRepo (e.g., articleRepo.adminDirty,
articleRepo.someMethod) to safely access the repo via optional chaining or
guards (this.articleRepo?.adminDirty or if (!this.articleRepo) return) and
ensure RepositoryObserver creation/disconnection still handles undefined repo.
In `@frontend/src/pages/page-admin-profile.ts`:
- Around line 199-204: The current JSON.parse usages for skills, certifications,
experiences, and links can throw if the stored strings are corrupted; add a
small defensive parser (e.g., safeParseArray<T>(value: string | null, fallback:
T[] = []): T[]) and replace the direct JSON.parse(...) calls in the object
creation with safeParseArray to return a fallback empty array on parse failure;
reference the fields names skills, certifications, experiences, links and ensure
any call sites that assume setFormValue-created values use the safe parser to
avoid runtime exceptions.
---
Duplicate comments:
In `@frontend/src/components/admin/ui/me-text-input.ts`:
- Around line 21-24: The host element me-text-input currently calls
setFormValue() but doesn't mirror the internal <input>'s validity into its
ElementInternals, so required/type="email" never blocks form submission; update
the component to call ElementInternals.setValidity() with the internal input's
validity state and validationMessage whenever the internal <input> changes
(input/change/blur) and inside setFormValue(), so invalid email or empty
required token sets host validity and prevents submit. Use the existing
ElementInternals instance (from attachInternals()) and read the internal
input.validity and input.validationMessage to construct the setValidity() call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 638d7810-6b6a-4d6c-b31a-90eec36f7663
📒 Files selected for processing (16)
frontend/src/components/admin/profile/me-profile-certifications-editor.tsfrontend/src/components/admin/profile/me-profile-experiences-editor.tsfrontend/src/components/admin/profile/me-profile-links-editor.tsfrontend/src/components/admin/profile/me-profile-skills-editor.tsfrontend/src/components/admin/ui/me-select.tsfrontend/src/components/admin/ui/me-text-input.tsfrontend/src/components/admin/ui/me-textarea.tsfrontend/src/components/app-admin-shell.tsfrontend/src/components/app-root.tsfrontend/src/controllers/RepositoryObserver.tsfrontend/src/pages/page-about.tsfrontend/src/pages/page-admin-account.tsfrontend/src/pages/page-admin-articles.tsfrontend/src/pages/page-admin-login.tsfrontend/src/pages/page-admin-profile.tsfrontend/src/pages/page-top.ts
Comment on lines
+42
to
+55
| private addItem = () => { | ||
| const next = [...this.links, { platform: '', url: '' }] | ||
| this.dispatchChange(next) | ||
| } | ||
|
|
||
| private removeItem(index: number) { | ||
| const next = this.links.filter((_, i) => i !== index) | ||
| this.dispatchChange(next) | ||
| } | ||
|
|
||
| private updateItem(index: number, patch: Partial<MeLink>) { | ||
| const next = [...this.links] | ||
| next[index] = { ...next[index], ...patch } | ||
| this.dispatchChange(next) |
Contributor
There was a problem hiding this comment.
「必須」と表示しているのに空リンクをそのまま保存できます。
addItem() は空の link を追加し、dispatchChange() は未入力の platform / url もそのまま JSON 化しています。さらに frontend/src/pages/page-admin-profile.ts Line 192-210 ではそれをそのまま保存しているので、不完全なリンク項目が公開データに混ざります。少なくとも送信値へ反映する前に必須チェックを入れてください。
Also applies to: 60-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/profile/me-profile-links-editor.ts` around
lines 42 - 55, The links editor currently allows empty link items to be added
and propagated because addItem() creates {platform:'', url:''} and
dispatchChange(...) serializes them; fix this by validating/sanitizing link
items before propagating or saving: update addItem/removeItem/updateItem or the
shared dispatchChange/submit path to trim fields and either prevent adding empty
items or filter out entries where platform or url are empty (check
MeLink.platform and MeLink.url), and ensure the save handler in
page-admin-profile.ts validates/filters links again before persisting so no
incomplete link objects reach storage.
Comment on lines
+23
to
+26
| formResetCallback() { | ||
| this.value = '' | ||
| this._internals.setFormValue('') | ||
| } |
Contributor
There was a problem hiding this comment.
formResetCallback のリセット値が意図した動作か確認してください。
フォームリセット時に常に空文字列にリセットしていますが、<option value="">すべて</option> のようなデフォルトオプションがある場合は問題ありません。ただし、最初のオプションが非空の値を持つ場合（例：<option value="qiita">Qiita</option>）、視覚的には最初のオプションが選択されますが、フォーム値は空文字列のままとなり、不整合が生じる可能性があります。
💡 改善案: 最初のオプション値を取得してリセット
formResetCallback() {
- this.value = ''
- this._internals.setFormValue('')
+ const select = this.shadowRoot?.querySelector('select')
+ const firstOptionValue = select?.options[0]?.value ?? ''
+ this.value = firstOptionValue
+ this._internals.setFormValue(firstOptionValue)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| formResetCallback() { | |
| this.value = '' | |
| this._internals.setFormValue('') | |
| } | |
| formResetCallback() { | |
| const select = this.shadowRoot?.querySelector('select') | |
| const firstOptionValue = select?.options[0]?.value ?? '' | |
| this.value = firstOptionValue | |
| this._internals.setFormValue(firstOptionValue) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ui/me-select.ts` around lines 23 - 26, The
formResetCallback currently forces this.value and this._internals.setFormValue
to empty string; change it to derive the reset value from the component's
options so form state and visual selection stay consistent: in
formResetCallback() get the first option's value (e.g., from this.options or the
rendered <option> list) and use that value if present, otherwise fall back to ''
and then set both this.value and this._internals.setFormValue(resetValue);
ensure you reference the existing formResetCallback method and the component's
options collection (e.g., this.options or whatever property holds option
elements) when implementing the change.
Comment on lines
+61
to
+70
| <select | ||
| id=${this._inputId} | ||
| .name=${this.name} | ||
| .value=${this.value ?? ''} | ||
| ?disabled=${this.disabled} | ||
| ?required=${this.required} | ||
| @change=${this._onChange} | ||
| > | ||
| <slot></slot> | ||
| </select> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Lit slot inside native select element compatibility
💡 Result:
Using a <slot> element inside a native <select> element in a Lit web component (or any Shadow DOM context) is not compatible with browsers. Native <select> elements only accept direct <option> and <optgroup> children per the HTML specification. The <slot> breaks this required parent-child relationship, causing slotted content (like <option>s) to not render or function properly in the select. This limitation holds across all major browsers (Chrome, Firefox, Safari, Edge) as of 2026-04-26. No CanIUse entry exists for select+slot support, and WHATWG issues (#11535, #7560) discussing flat-tree traversal for options remain open without implementation. Workarounds in Lit: - Listen for 'slotchange' on a hidden <slot>, clone or move assigned <option>s into the native <select> using slot.assignedElements or assignedNodes. - Build a fully custom select using divs/overlays for complete control, avoiding native <select>. - Use customized built-in elements extending <select> (no Shadow DOM). Recent WHATWG changes relax <select> parser/content model for customizable selects (e.g., allowing more content in <option>), but do not enable direct <slot> support inside <select>.
Citations:
- 1: https://stackoverflow.com/questions/57878970/using-slot-in-native-select-element
- 2: Pseudo-native select using litelement lit/lit-element#951
- 3: https://stackoverflow.com/questions/77163649/lit-not-rendering-slotted-children
- 4: The select element doesn't support slotted option elements whatwg/html#11535
- 5: https://stackoverflow.com/questions/56351274/how-to-pass-option-tags-to-a-custom-element-as-distributed-nodes-aka-slot-sl
- 6: Suggestion - Allow "<slot>" to provide options for select or datalist whatwg/html#7560
- 7: Relax
<select>parser whatwg/html#10557
🏁 Script executed:
fd -t f "me-select.ts" frontend/Repository: umekikazuya/me
Length of output: 104
🏁 Script executed:
cat -n frontend/src/components/admin/ui/me-select.ts | sed -n '55,75p'Repository: umekikazuya/me
Length of output: 720
ネイティブ <select> 要素内での <slot> 使用は機能しません。
ネイティブ <select> 要素は HTML 仕様に従い、<option> と <optgroup> の直接の子要素のみを受け入れます。<slot> を使用するとこの必須の親子関係が破壊され、スロットされた <option> 要素は Chrome、Firefox、Safari、Edge を含むすべての主要ブラウザでレンダリングおよび機能しなくなります。
代わりに以下のいずれかの対応を検討してください：
slotchangeイベントをリッスンして、割り当てられた<option>要素をslot.assignedElements()を使用して直接<select>に移動する- カスタム
<select>を完全に構築する（div/オーバーレイを使用） - Shadow DOM を使用しないカスタマイズ済み組み込み要素を使用する
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ui/me-select.ts` around lines 61 - 70, ネイティブ
<select> 内に <slot> を置くとブラウザで機能しないので、テンプレートの <select id=${this._inputId} ...> から
<slot> を削除し、代わりにスロット要素（例: クラス/プロパティにある slot 要素）で 'slotchange' を監視して
slot.assignedElements() から返された <option> 要素を実際の <select>（識別子: this._inputId /
要素参照）へ移動/appendChild する処理を実装してください；既存のハンドラ/プロパティ（_onChange, name, value,
disabled, required）はそのまま維持して select に適用してください。
Comment on lines
+13
to
+15
| @property({ type: Boolean }) disabled = false | ||
| @property({ type: Boolean }) required = false | ||
| @property() placeholder = '' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For a form-associated custom element using ElementInternals, does a required <textarea> inside shadow DOM automatically participate in ancestor form validation, or must the host mirror validity via ElementInternals.setValidity()?
💡 Result:
For a form-associated custom element using ElementInternals, a required <textarea> inside shadow DOM does not automatically participate in ancestor form validation. The host must mirror the inner textarea's validity state to its ElementInternals instance using ElementInternals.setValidity (typically by syncing the textarea's .validity object, along with .validationMessage and the textarea as the anchor element).
Citations:
- 1: https://frontendmasters.com/blog/form-associated-custom-elements-in-practice/
- 2: https://dev.to/stuffbreaker/custom-forms-with-web-components-and-elementinternals-4jaj
- 3: https://stackoverflow.com/questions/76862158/validating-a-form-associated-custom-element
- 4: https://frontendmasters.com/blog/form-associated-custom-elements-in-practice
🏁 Script executed:
fd 'me-textarea.ts' --type fRepository: umekikazuya/me
Length of output: 106
🏁 Script executed:
cat -n frontend/src/components/admin/ui/me-textarea.tsRepository: umekikazuya/me
Length of output: 3855
required が親フォームの検証に反映されていません。
setFormValue() で値は更新していますが、host 要素の validity 状態は一度も同期していません。内部 <textarea> には ?required=${this.required} で required 属性を設定していますが、これだけでは shadow DOM 内のみの検証に留まり、祖先フォームの perspective からは常に有効と判定されます。その結果、<me-textarea required> を使っても親フォームは空入力を無効扱いできません。
_onInput() メソッドおよび updated() メソッドで、内部 textarea の .validity および .validationMessage を ElementInternals.setValidity() に反映してください。textarea を anchor 要素として指定し、その validity 状態を host に伝播させる必要があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ui/me-textarea.ts` around lines 13 - 15, 内部
textarea の検証状態がホストに伝播されておらず親フォームに反映されないため、_onInput() と updated() 内で内部 textarea の
validity と validationMessage を ElementInternals.setValidity() に反映するよう実装してください:
コンポーネントの ElementInternals を取得（this.attachInternals() を既に使っていなければ初期化）し、内部
<textarea> をアンカー要素として扱いその validity（valid, validity object）と validationMessage
を基に setValidity() を呼び出して host の状態を更新し、また setFormValue()
呼び出し後に必ず同期するよう修正してください（参照シンボル: required プロパティ、setFormValue(), _onInput(),
updated(), ElementInternals.setValidity(), textarea をアンカー要素として指定）。
Comment on lines
+11
to
+37
| constructor(host: ReactiveControllerHost, repository: EventTarget) { | ||
| this._host = host | ||
| this._repository = repository | ||
| host.addController(this) | ||
| } | ||
|
|
||
| hostConnected() { | ||
| this.connect() | ||
| } | ||
|
|
||
| hostDisconnected() { | ||
| this.disconnect() | ||
| } | ||
|
|
||
| /** | ||
| * Manually start observing the repository. | ||
| */ | ||
| connect() { | ||
| this._repository.addEventListener('change', this._onRepositoryChange) | ||
| } | ||
|
|
||
| /** | ||
| * Manually stop observing the repository. | ||
| */ | ||
| disconnect() { | ||
| this._repository.removeEventListener('change', this._onRepositoryChange) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Lit, if a ReactiveController is added with addController(), does it keep receiving hostConnected()/hostDisconnected() on future host reconnects until removeController() is called? Also, does ReactiveControllerHost expose removeController() for cleanup?
💡 Result:
Yes, a ReactiveController added with addController continues to receive hostConnected and hostDisconnected callbacks on future host reconnects (i.e., every time the host's connectedCallback/disconnectedCallback is invoked) until removeController is explicitly called. This is because Lit's ReactiveElement implementation loops over its registered controllers (stored in this.controllers Set) in both connectedCallback — calling c.hostConnected?. — and disconnectedCallback — calling c.hostDisconnected?. — without any state tracking that would skip calls after the first connection. Yes, ReactiveControllerHost exposes removeController(controller: ReactiveController): void explicitly for removing a controller from the host, stopping its lifecycle callbacks. LitElement/ReactiveElement implements this method, adding the controller to a Set on addController (with immediate hostConnected if already connected) and removing it on removeController.
Citations:
- 1: https://github.com/lit/lit/blob/main/packages/reactive-element/src/reactive-element.ts
- 2: https://github.com/lit/lit/blob/main/packages/reactive-element/src/reactive-controller.ts
- 3: https://lit.dev/docs/api/controllers/
- 4: https://lit.dev/docs/composition/controllers/
- 5: https://mintlify.com/lit/lit/api/reactive-controller
🏁 Script executed:
# Find the page files mentioned in the review
fd -t f "page-about.ts|page-top.ts|page-admin-account.ts" frontend/src/pages/Repository: umekikazuya/me
Length of output: 163
🏁 Script executed:
# Check page-about.ts around lines 15-16
cat -n frontend/src/pages/page-about.ts | sed -n '10,25p'Repository: umekikazuya/me
Length of output: 696
🏁 Script executed:
# Check page-top.ts around lines 18-19
cat -n frontend/src/pages/page-top.ts | sed -n '13,30p'Repository: umekikazuya/me
Length of output: 726
🏁 Script executed:
# Check page-admin-account.ts around lines 16-17
cat -n frontend/src/pages/page-admin-account.ts | sed -n '10,25p'Repository: umekikazuya/me
Length of output: 682
🏁 Script executed:
# Check the RepositoryObserver.ts file for _onRepositoryChange handler
cat -n frontend/src/controllers/RepositoryObserver.tsRepository: umekikazuya/me
Length of output: 1369
古い observer が host に登録され続け、再接続時に再購読されます。
disconnect() は repository の listener のみを削除するため、page-about.ts (L15-16)、page-top.ts (L18-19)、page-admin-account.ts (L16-17) では observer を差し替えても、古い controller 自体は host に登録されたままです。要素が再接続されると stale controller の hostConnected() が呼び出され、古い repository へ listener が再登録されます。
disconnect() とは別に host からも外す破棄メソッドを実装し、差し替え側で呼び出してください：
修正案
export class RepositoryObserver implements ReactiveController {
private _host: ReactiveControllerHost
private _repository: EventTarget
+ private _connected = false
constructor(host: ReactiveControllerHost, repository: EventTarget) {
this._host = host
this._repository = repository
host.addController(this)
@@
connect() {
+ if (this._connected) return
this._repository.addEventListener('change', this._onRepositoryChange)
+ this._connected = true
}
@@
disconnect() {
+ if (!this._connected) return
this._repository.removeEventListener('change', this._onRepositoryChange)
+ this._connected = false
+ }
+
+ dispose() {
+ this.disconnect()
+ this._host.removeController(this)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/controllers/RepositoryObserver.ts` around lines 11 - 37,
RepositoryObserver currently only removes the repository event listener in
disconnect(), leaving the controller registered on the host so stale controllers
get reconnected; add a public dispose/destroy method on RepositoryObserver that
calls this.disconnect() and also removes the controller from the host (call
host.removeController(this) or the equivalent on ReactiveControllerHost), and
update replacement sites to call that dispose method when swapping observers;
reference the class and its methods (constructor, hostConnected,
hostDisconnected, connect, disconnect, add a new dispose/destroy) so you remove
the stale controller from the host as part of cleanup.
Comment on lines
+49
to
+60
| @consume({ context: articleContext, subscribe: true }) | ||
| set articleRepo(repo: IArticleRepository) { | ||
| if (this._articleRepo === repo) return | ||
| this._articleRepo = repo | ||
| if (this._observer) this._observer.disconnect() | ||
| if (repo) this._observer = new RepositoryObserver(this, repo) | ||
| } | ||
| get articleRepo() { | ||
| return this._articleRepo | ||
| } | ||
| private _articleRepo!: IArticleRepository | ||
| private _observer?: RepositoryObserver |
Contributor
There was a problem hiding this comment.
_articleRepo の definite assignment assertion はランタイムエラーを隠す可能性があります。
セッターでは if (repo) で undefined を防御していますが、Line 59 の private _articleRepo!: IArticleRepository は型システムを迂回しています。コンテキストプロバイダーが存在しない場合や初回 render 時に this.articleRepo.adminDirty（Line 102, 423, 451 等）がアクセスされると実行時エラーになります。
_authRepo?: IAuthRepository のように optional 型にし、参照箇所で optional chaining を使用することを推奨します。
🛡️ 修正案
- private _articleRepo!: IArticleRepository
+ private _articleRepo?: IArticleRepositoryLine 102 などの参照箇所:
- if (!this.articleRepo.adminDirty && !this.localDirty) return
+ if (!this.articleRepo?.adminDirty && !this.localDirty) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/page-admin-articles.ts` around lines 49 - 60, The field
_articleRepo currently uses a definite assignment assertion which can hide
runtime errors; change private _articleRepo!: IArticleRepository to an optional
field (private _articleRepo?: IArticleRepository) and keep the articleRepo
setter/getter logic but accept undefined; update all usages of articleRepo
(e.g., articleRepo.adminDirty, articleRepo.someMethod) to safely access the repo
via optional chaining or guards (this.articleRepo?.adminDirty or if
(!this.articleRepo) return) and ensure RepositoryObserver creation/disconnection
still handles undefined repo.
Comment on lines
+629
to
+631
| this.setBaseline(cloneArticleDraft(draft)) | ||
| await Promise.all([this.reloadArticles(), this.refreshTags()]) | ||
| this.localDirty = false |
Contributor
There was a problem hiding this comment.
保存成功後に articleRepo.adminDirty がリセットされていません。
Line 631 で localDirty = false をセットしていますが、articleRepo.setAdminDirty(false) を呼んでいません。ProfileRepository は saveAdminProfile() 内で自動的に adminDirty をクリアしますが（関連スニペット参照）、ArticleRepository は単純なセッターのみで自動クリアロジックがありません。
この結果、保存成功後も Line 423-424 の dirty indicator が「未保存の変更があります」のままになります。
🐛 修正案
this.setBaseline(cloneArticleDraft(draft))
await Promise.all([this.reloadArticles(), this.refreshTags()])
this.localDirty = false
+ this.articleRepo.setAdminDirty(false)
} catch (error) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.setBaseline(cloneArticleDraft(draft)) | |
| await Promise.all([this.reloadArticles(), this.refreshTags()]) | |
| this.localDirty = false | |
| this.setBaseline(cloneArticleDraft(draft)) | |
| await Promise.all([this.reloadArticles(), this.refreshTags()]) | |
| this.localDirty = false | |
| this.articleRepo.setAdminDirty(false) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/page-admin-articles.ts` around lines 629 - 631, After
successfully saving the draft (after this.setBaseline(cloneArticleDraft(draft))
and awaiting Promise.all([...]) ), also clear the repository-level dirty flag by
calling articleRepo.setAdminDirty(false) so the global dirty indicator updates;
locate the save flow around
setBaseline/cloneArticleDraft/reloadArticles/refreshTags and add
articleRepo.setAdminDirty(false) immediately after setting this.localDirty =
false (or instead of) to ensure the adminDirty state is reset like
ProfileRepository.saveAdminProfile does.
Comment on lines
+199
to
+204
| skills: JSON.parse((formData.get('skills') as string) || '[]'), | ||
| certifications: JSON.parse( | ||
| (formData.get('certifications') as string) || '[]', | ||
| ), | ||
| experiences: JSON.parse((formData.get('experiences') as string) || '[]'), | ||
| links: JSON.parse((formData.get('links') as string) || '[]'), |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
JSON.parse に対する防御的エラーハンドリングの検討。
form-associated コンポーネントは常に有効な JSON を setFormValue で設定しますが、万が一不正な値が混入した場合に JSON.parse が例外をスローする可能性があります。現状ではリスクは低いですが、防御的コーディングとして try-catch を検討してもよいでしょう。
💡 防御的パース関数の例
const safeParseArray = <T>(value: string | null, fallback: T[] = []): T[] => {
try {
return JSON.parse(value || '[]')
} catch {
return fallback
}
}
// 使用例
skills: safeParseArray<MeSkill>(formData.get('skills') as string),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/page-admin-profile.ts` around lines 199 - 204, The current
JSON.parse usages for skills, certifications, experiences, and links can throw
if the stored strings are corrupted; add a small defensive parser (e.g.,
safeParseArray<T>(value: string | null, fallback: T[] = []): T[]) and replace
the direct JSON.parse(...) calls in the object creation with safeParseArray to
return a fallback empty array on parse failure; reference the fields names
skills, certifications, experiences, links and ensure any call sites that assume
setFormValue-created values use the safe parser to avoid runtime exceptions.
umekikazuya
commented
Apr 26, 2026
Owner
Author
umekikazuya
left a comment
umekikazuya
left a comment
There was a problem hiding this comment.
確認済み。一部レビュー内容は次PRで対応。
This was referenced Apr 27, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary