Moved to kebab-case formatting signup-form#43
Conversation
- renamed files to kebab-case Changelog for v0.3.2 -> 0.3.3:
WalkthroughThis pull request standardizes file naming conventions across the signup-form application by converting filenames from PascalCase to lowercase-hyphenated format (kebab-case), updates all corresponding import paths throughout the codebase, adds ESLint rules to enforce this convention, includes minor logic adjustments for email handling and loading states, and extends the URLHistory type with an optional path field. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/signup-form/src/components/pages/form-page.tsx (1)
27-30: Clear loading state in minimal success path.In minimal mode,
loadingstaystrueafter success, leaving the form in a perpetual loading state.🐛 Proposed fix
if (minimal) { // Don't go to the success page, but show the success state in the form + setLoading(false); setSuccess(true); } else { setPage('SuccessPage', { email });
🤖 Fix all issues with AI agents
In `@apps/signup-form/README.md`:
- Line 62: The README sentence ending with "core/shared/config/default.json" is
truncated; complete the sentence to state the exact action and context (e.g.,
"To use the new version of the signup form in Ghost, update the signup form
version value in Ghost core's default configuration file at
core/shared/config/default.json and restart Ghost."). Also ensure proper
punctuation and, if helpful, wrap the file path in code formatting (backticks)
and add a brief example config key (e.g., "signupForm.version") or a pointer to
the config section so readers know what to change.
In `@apps/signup-form/src/utils/helpers.tsx`:
- Around line 44-46: getUrlHistory currently returns the raw value from
getDefaultUrlHistory which can be undefined, violating its declared URLHistory
return type; update getUrlHistory to guard the result of getDefaultUrlHistory
(call getDefaultUrlHistory() inside getUrlHistory) and return an empty array
fallback when the result is falsy or not an array so the function always returns
a URLHistory; reference getUrlHistory and getDefaultUrlHistory to locate and
change the return path.
🧹 Nitpick comments (2)
apps/signup-form/README.md (1)
46-60: Consider clarifying the post-release step for patch releases.The post-release step to update Ghost core's
default.jsonis only mentioned under the minor/major section (line 62), but patch releases would also require this update to take effect in Ghost. Consider either:
- Moving this note to a shared "Post-release" section applicable to all release types, or
- Adding this step explicitly under the patch release section as well.
apps/signup-form/src/utils/helpers.tsx (1)
20-20: Minor: Quote style inconsistency with related files.The
STORAGE_KEYnow uses double quotes, but related files in the codebase (apps/portal/src/utils/helpers.jsandghost/core/core/frontend/src/member-attribution/member-attribution.js) use single quotes for the same constant. This is a minor consistency concern and may be governed by ESLint rules.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/signup-form/.eslintrc.cjsapps/signup-form/.storybook/preview.tsxapps/signup-form/README.mdapps/signup-form/package.jsonapps/signup-form/src/app-context.tsapps/signup-form/src/app.tsxapps/signup-form/src/components/content-box.tsxapps/signup-form/src/components/frame.tsxapps/signup-form/src/components/iframe.tsxapps/signup-form/src/components/pages/form-page.tsxapps/signup-form/src/components/pages/form-view.stories.tsapps/signup-form/src/components/pages/form-view.tsxapps/signup-form/src/components/pages/success-page.tsxapps/signup-form/src/components/pages/success-view.stories.tsapps/signup-form/src/components/pages/success-view.tsxapps/signup-form/src/index.tsxapps/signup-form/src/pages.tsxapps/signup-form/src/preview.stories.tsxapps/signup-form/src/utils/helpers.tsxapps/signup-form/src/utils/options.tsxapps/signup-form/test/utils/is-test-env.jsghost/core/core/shared/config/defaults.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/signup-form/src/utils/helpers.tsx (2)
ghost/core/core/frontend/src/member-attribution/member-attribution.js (2)
STORAGE_KEY(8-8)history(43-43)apps/portal/src/utils/helpers.js (1)
STORAGE_KEY(895-895)
🔇 Additional comments (17)
ghost/core/core/shared/config/defaults.json (1)
305-305: LGTM: Formatting consistency improved.Removing the space before the colon aligns this key with standard JSON formatting and matches the rest of the file's style.
apps/signup-form/.eslintrc.cjs (1)
23-25: Filename regex aligns with the kebab‑case goal.Looks good and still permits dotfiles/extensions as needed.
apps/signup-form/.storybook/preview.tsx (1)
6-6: Import path update is correct and complete.The import path has been successfully updated to reflect the kebab-case naming convention (
app-context), and bothAppContextProviderandAppContextTypeare properly exported from the new file. No lingering references to the old path remain in the codebase.apps/signup-form/src/utils/options.tsx (1)
2-2: LGTM!The import path update from
'../AppContext'to'../app-context'correctly reflects the kebab-case file renaming convention applied across the project.apps/signup-form/package.json (1)
3-3: LGTM!Patch version bump to
0.3.3is appropriate for this non-breaking refactor that renames files to kebab-case.apps/signup-form/src/components/frame.tsx (1)
1-5: LGTM!Import paths correctly updated to reflect kebab-case file naming:
'./IFrame'→'./iframe''../AppContext'→'../app-context'apps/signup-form/src/components/pages/success-view.tsx (1)
3-3: LGTM!Import path correctly updated from
'../../AppContext'to'../../app-context'to match the kebab-case naming convention.apps/signup-form/src/components/pages/success-view.stories.ts (1)
3-3: LGTM!Import path correctly updated from
'./SuccessView'to'./success-view'to match the kebab-case file naming convention.apps/signup-form/src/preview.stories.tsx (1)
4-5: LGTM!Import paths correctly updated to reflect kebab-case file naming:
'./AppContext'→'./app-context''./components/ContentBox'→'./components/content-box'apps/signup-form/src/app.tsx (1)
4-6: LGTM!The import paths have been correctly updated to use kebab-case formatting (
app-context,content-box,frame), consistent with the PR's file renaming convention.apps/signup-form/src/index.tsx (1)
1-1: LGTM!The import path has been correctly updated to reference the renamed
app.tsxfile.apps/signup-form/src/utils/helpers.tsx (1)
1-1: LGTM!Import path correctly updated to kebab-case convention.
apps/signup-form/src/components/pages/form-view.tsx (2)
3-3: LGTM!Import path correctly updated to kebab-case convention.
54-57: Good addition: Trimming email input.Trimming the email before submission is a sensible improvement that prevents accidental whitespace from causing validation failures or storing malformed email addresses.
apps/signup-form/src/components/pages/form-view.stories.ts (1)
3-3: LGTM!Import path correctly updated to reference the renamed
form-viewmodule.apps/signup-form/src/components/pages/success-page.tsx (1)
2-3: Kebab-case import update looks good.Aligns with the renamed module paths; no behavioral impact.
apps/signup-form/src/pages.tsx (1)
2-3: Import path normalization looks good.Matches the new kebab-case module naming.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| 2. Merge the release commit to `main` | ||
| 3. Wait until a new version of Ghost is released | ||
|
|
||
| To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`) |
There was a problem hiding this comment.
Documentation appears truncated.
Line 62 ends abruptly with a file path reference but no closing context. Consider completing the sentence or adding proper formatting:
-To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`)
+To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`).📝 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.
| To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`) | |
| To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`). |
🤖 Prompt for AI Agents
In `@apps/signup-form/README.md` at line 62, The README sentence ending with
"core/shared/config/default.json" is truncated; complete the sentence to state
the exact action and context (e.g., "To use the new version of the signup form
in Ghost, update the signup form version value in Ghost core's default
configuration file at core/shared/config/default.json and restart Ghost."). Also
ensure proper punctuation and, if helpful, wrap the file path in code formatting
(backticks) and add a brief example config key (e.g., "signupForm.version") or a
pointer to the config section so readers know what to change.
| const history = getDefaultUrlHistory(); | ||
| if (history) { | ||
| return history; | ||
| } | ||
| return history; | ||
| } |
There was a problem hiding this comment.
Potential undefined return when history is not found.
getDefaultUrlHistory() can return undefined (when sessionStorage is empty or on error), but getUrlHistory has a return type of URLHistory (an array). This change removes any guard and directly returns the potentially undefined value.
Consider adding a fallback to return an empty array:
Suggested fix
if (window.location.host === new URL(siteUrl).host) {
const history = getDefaultUrlHistory();
- return history;
+ return history ?? [];
}📝 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.
| const history = getDefaultUrlHistory(); | |
| if (history) { | |
| return history; | |
| } | |
| return history; | |
| } | |
| if (window.location.host === new URL(siteUrl).host) { | |
| const history = getDefaultUrlHistory(); | |
| return history ?? []; | |
| } |
🤖 Prompt for AI Agents
In `@apps/signup-form/src/utils/helpers.tsx` around lines 44 - 46, getUrlHistory
currently returns the raw value from getDefaultUrlHistory which can be
undefined, violating its declared URLHistory return type; update getUrlHistory
to guard the result of getDefaultUrlHistory (call getDefaultUrlHistory() inside
getUrlHistory) and return an empty array fallback when the result is falsy or
not an array so the function always returns a URLHistory; reference
getUrlHistory and getDefaultUrlHistory to locate and change the return path.
2 participants
Benchmark PR from agentic-review-benchmarks#5
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.