-
-
Notifications
You must be signed in to change notification settings - Fork 11.5k
Polished offers UI with consistent styling and improved navigation #26462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
636c392
b43e5d0
2cd753a
825669c
a632760
27508b7
e64335a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -126,7 +126,7 @@ const Sidebar: React.FC<{ | |||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||
| </section> | ||||||||||||||||||||||||||||||||||||||
| <section className='mt-4'> | ||||||||||||||||||||||||||||||||||||||
| <section className='mt-2'> | ||||||||||||||||||||||||||||||||||||||
| <h2 className='mb-4 text-lg'>General</h2> | ||||||||||||||||||||||||||||||||||||||
| <div className='flex flex-col gap-6'> | ||||||||||||||||||||||||||||||||||||||
| <TextField | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -255,6 +255,16 @@ const EditOfferModal: React.FC<{id: string}> = ({id}) => { | |||||||||||||||||||||||||||||||||||||
| portalParent='offers' | ||||||||||||||||||||||||||||||||||||||
| />; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const goBack = () => { | ||||||||||||||||||||||||||||||||||||||
| if (sessionStorage.getItem('editOfferPageSource') === 'offers') { | ||||||||||||||||||||||||||||||||||||||
| sessionStorage.removeItem('editOfferPageSource'); | ||||||||||||||||||||||||||||||||||||||
| updateRoute('offers'); | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| sessionStorage.removeItem('editOfferPageSource'); | ||||||||||||||||||||||||||||||||||||||
| updateRoute('offers/edit'); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+258
to
+266
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong default route when
🐛 Proposed fix const goBack = () => {
+ sessionStorage.removeItem('editOfferPageSource');
if (sessionStorage.getItem('editOfferPageSource') === 'offers') {
- sessionStorage.removeItem('editOfferPageSource');
updateRoute('offers');
} else {
- sessionStorage.removeItem('editOfferPageSource');
- updateRoute('offers/edit');
+ updateRoute('offers');
}
};If 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents🧩 Analysis chain🏁 Script executed: rg -n "editOfferPageSource" --type=ts --type=tsx -A 2 -B 2Repository: TryGhost/Ghost Length of output: 85 🏁 Script executed: rg -n "editOfferPageSource" -A 2 -B 2Repository: TryGhost/Ghost Length of output: 2905 🏁 Script executed: rg "offers/edit" --type-not test -A 1 -B 1Repository: TryGhost/Ghost Length of output: 86 🏁 Script executed: rg "offers/edit" -A 1 -B 1 | head -80Repository: TryGhost/Ghost Length of output: 7373 Unguarded navigation paths to Multiple code paths navigate to Replace sessionStorage-based routing state with route query parameters or a scoped React context to eliminate the coupling between multiple entry points and the goBack handler. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return offerById ? <PreviewModalContent | ||||||||||||||||||||||||||||||||||||||
| afterClose={() => { | ||||||||||||||||||||||||||||||||||||||
| updateRoute('offers'); | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -267,21 +277,17 @@ const EditOfferModal: React.FC<{id: string}> = ({id}) => { | |||||||||||||||||||||||||||||||||||||
| okColor={okProps.color} | ||||||||||||||||||||||||||||||||||||||
| okLabel={okProps.label || 'Save'} | ||||||||||||||||||||||||||||||||||||||
| preview={iframe} | ||||||||||||||||||||||||||||||||||||||
| previewToolbar={false} | ||||||||||||||||||||||||||||||||||||||
| previewToolbarBreadcrumbs={[ | ||||||||||||||||||||||||||||||||||||||
| {label: 'Offers', onClick: goBack}, | ||||||||||||||||||||||||||||||||||||||
| {label: formState?.name || 'Offer'} | ||||||||||||||||||||||||||||||||||||||
| ]} | ||||||||||||||||||||||||||||||||||||||
| sidebar={sidebar} | ||||||||||||||||||||||||||||||||||||||
| size='lg' | ||||||||||||||||||||||||||||||||||||||
| testId='offer-update-modal' | ||||||||||||||||||||||||||||||||||||||
| title='Offer' | ||||||||||||||||||||||||||||||||||||||
| width={1140} | ||||||||||||||||||||||||||||||||||||||
| onCancel={() => { | ||||||||||||||||||||||||||||||||||||||
| if (sessionStorage.getItem('editOfferPageSource') === 'offers') { | ||||||||||||||||||||||||||||||||||||||
| sessionStorage.removeItem('editOfferPageSource'); | ||||||||||||||||||||||||||||||||||||||
| updateRoute('offers'); | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| sessionStorage.removeItem('editOfferPageSource'); | ||||||||||||||||||||||||||||||||||||||
| updateRoute('offers/edit'); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||
| onBreadcrumbsBack={goBack} | ||||||||||||||||||||||||||||||||||||||
| onCancel={goBack} | ||||||||||||||||||||||||||||||||||||||
| onOk={async () => { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| if (await handleSave({force: true})) { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,7 +51,7 @@ const OffersRetention: React.FC = () => { | |||||||||||||||
| <col className='w-[80px]' /> | ||||||||||||||||
| </colgroup> | ||||||||||||||||
| {placeholderRetentionOffers.map(offer => ( | ||||||||||||||||
| <tr key={offer.id} className='group relative scale-100 border-b border-b-grey-200 dark:border-grey-800' data-testid='retention-offer-item'> | ||||||||||||||||
| <tr key={offer.id} className='group relative scale-100 border-b border-b-grey-100 dark:border-b-grey-950' data-testid='retention-offer-item'> | ||||||||||||||||
| <td className='p-0'> | ||||||||||||||||
| <a className='block cursor-pointer p-5 pl-0' onClick={() => handleRetentionOfferClick(offer.id)}> | ||||||||||||||||
| <span className='font-semibold'>{offer.name}</span><br /> | ||||||||||||||||
|
|
@@ -78,9 +78,9 @@ const OffersRetention: React.FC = () => { | |||||||||||||||
| <td className='whitespace-nowrap p-0 text-sm'> | ||||||||||||||||
| <a className='block cursor-pointer p-5' onClick={() => handleRetentionOfferClick(offer.id)}> | ||||||||||||||||
| {offer.status === 'active' ? ( | ||||||||||||||||
| <span className='text-sm font-semibold text-green'>Active</span> | ||||||||||||||||
| <span className='inline-flex items-center rounded-full bg-[rgba(48,207,67,0.15)] px-1.5 py-0.5 text-2xs font-semibold uppercase tracking-wide text-green'>Active</span> | ||||||||||||||||
| ) : ( | ||||||||||||||||
| <span className='text-sm text-grey-700'>Inactive</span> | ||||||||||||||||
| <span className='inline-flex items-center rounded-full bg-grey-200 px-1.5 py-0.5 text-2xs font-semibold uppercase tracking-wide text-grey-700 dark:bg-grey-900 dark:text-grey-500'>Inactive</span> | ||||||||||||||||
|
Comment on lines
+81
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Find Tailwind config and design-system files
find . -type f -name 'tailwind.config.*' -o -name 'theme*' -o -name 'config*' | grep -E '(tailwind|theme|config)' | head -20Repository: TryGhost/Ghost Length of output: 1224 🏁 Script executed: # Search for text-2xs definition in the codebase
rg 'text-2xs|"2xs"' --type ts --type tsx --type js --type json -B 2 -A 2 | head -100Repository: TryGhost/Ghost Length of output: 85 🏁 Script executed: # Check the actual file to confirm the code snippet
cat -n apps/admin-x-settings/src/components/settings/growth/offers/offers-retention.tsx | sed -n '75,90p'Repository: TryGhost/Ghost Length of output: 1236 🏁 Script executed: # Look for similar badge patterns in the codebase
rg 'className.*inline-flex.*rounded-full' --type tsx --type ts -B 1 -A 1 | head -60Repository: TryGhost/Ghost Length of output: 85 🏁 Script executed: # Search for text-2xs definition without file type specification
rg 'text-2xs|"2xs"' -B 2 -A 2 | head -150Repository: TryGhost/Ghost Length of output: 5076 🏁 Script executed: # Check the admin-x-settings Tailwind config
cat apps/admin-x-settings/tailwind.config.cjsRepository: TryGhost/Ghost Length of output: 346 🏁 Script executed: # Check admin-x-design-system Tailwind config
cat apps/admin-x-design-system/tailwind.config.cjsRepository: TryGhost/Ghost Length of output: 11646 🏁 Script executed: # Search for fontSize or theme config with 2xs
rg 'fontSize|2xs' apps/admin-x-design-system/ -A 2 -B 2 | head -100Repository: TryGhost/Ghost Length of output: 3096 Active badge missing dark-mode background and text adjustments The 🎨 Suggested fix-<span className='inline-flex items-center rounded-full bg-[rgba(48,207,67,0.15)] px-1.5 py-0.5 text-2xs font-semibold uppercase tracking-wide text-green'>Active</span>
+<span className='inline-flex items-center rounded-full bg-[rgba(48,207,67,0.15)] dark:bg-[rgba(48,207,67,0.08)] px-1.5 py-0.5 text-2xs font-semibold uppercase tracking-wide text-green dark:text-green-400'>Active</span>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| )} | ||||||||||||||||
| </a> | ||||||||||||||||
| </td> | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation for
durationInMonthswhen duration isrepeating.Clearing this field stores
0(viaNumber('')), andonValidatehas no guard for it —duration_in_months: 0would be submitted to the API. Now that the field defaults to1and is more visible, this gap is easier to trigger.🛡️ Proposed fix — add guard in `onValidate`
if (formState.type === 'percent' && (formState.percentAmount < 0 || formState.percentAmount > 100)) { newErrors.amount = 'Amount must be between 0 and 100%.'; } +if (formState.duration === 'repeating' && (!formState.durationInMonths || formState.durationInMonths < 1)) { + newErrors.durationInMonths = 'Please enter a duration of at least 1 month.'; +}🤖 Prompt for AI Agents