Skip to content

fix(react): remove orange dot and prevent layout shift in formactions #68

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.

Sign up for GitHub

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

Open
harishsundar-okta wants to merge 3 commits into
base: main
Choose a base branch
from
+48 −26
Open

fix(react): remove orange dot and prevent layout shift in formactions #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b625634
fix(react): remove orange dot and prevent layout shift in formactions
harishsundar-okta Jan 30, 2026
78b4c26
fix(react): apply cls prevention only in edit mode
harishsundar-okta Jan 30, 2026
38bf4c7
fix(react): use tailwind class for min-width instead of arbitrary value
harishsundar-okta Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions ...ocks/my-organization/organization-management/__tests__/organization-details-edit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,6 @@ describe('OrganizationDetailsEdit', () => {
const saveButton = screen.getByRole('button', { name: /submit_button_label/i });
expect(saveButton).toBeDisabled();

const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
expect(cancelButton).toBeDisabled();

expect(displayNameInput).toHaveAttribute('readonly');
});
});
Expand Down Expand Up @@ -389,6 +386,7 @@ describe('OrganizationDetailsEdit', () => {
describe('cancelAction.disabled', () => {
describe('when is true', () => {
it('should disable cancel button', async () => {
const user = userEvent.setup();
const mockCancelAction = { ...createMockCancelAction(), disabled: true };

renderWithProviders(
Expand All @@ -397,16 +395,18 @@ describe('OrganizationDetailsEdit', () => {
/>,
);

await waitForComponentToLoad();
const displayNameInput = await waitForComponentToLoad();
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

// Button should is disabled
const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
expect(cancelButton).toBeDisabled();
});
});

describe('when is false', () => {
it('should enable cancel button', async () => {
const user = userEvent.setup();
const mockCancelAction = { ...createMockCancelAction(), disabled: false };

renderWithProviders(
Expand All @@ -415,7 +415,9 @@ describe('OrganizationDetailsEdit', () => {
/>,
);

await waitForComponentToLoad();
const displayNameInput = await waitForComponentToLoad();
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
expect(cancelButton).not.toBeDisabled();
Expand All @@ -435,9 +437,10 @@ describe('OrganizationDetailsEdit', () => {
/>,
);

await waitForComponentToLoad();
const displayNameInput = await waitForComponentToLoad();
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

// Click cancel
const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
await user.click(cancelButton);

Expand Down
32 changes: 24 additions & 8 deletions ...tion/organization-management/organization-details/__tests__/organization-details.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ describe('OrganizationDetails', () => {
describe('formActions', () => {
describe('formActions.isLoading', () => {
describe('when isLoading is true', () => {
it('should disable save and cancel buttons', () => {
it('should disable save and cancel buttons', async () => {
const user = userEvent.setup();
const mockFormActions = createMockFormActions({ isLoading: true });

renderWithProviders(
Expand All @@ -172,11 +173,12 @@ describe('OrganizationDetails', () => {
/>,
);

const buttons = screen.getAllByRole('button');
const saveButton = buttons.find((btn) => btn.classList.contains('FormActions-next'));
const cancelButton = buttons.find((btn) =>
btn.classList.contains('FormActions-previous'),
);
const displayNameInput = screen.getByLabelText(/display_name\.label/i);
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

const saveButton = document.querySelector('button[type="submit"]') as HTMLButtonElement;
const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });

expect(saveButton).toBeDisabled();
expect(cancelButton).toBeDisabled();
Expand Down Expand Up @@ -333,7 +335,8 @@ describe('OrganizationDetails', () => {
describe('formActions.previousAction', () => {
describe('formActions.previousAction.disabled', () => {
describe('when disabled is true', () => {
it('should disable cancel button', () => {
it('should disable cancel button', async () => {
const user = userEvent.setup();
const mockFormActions = createMockFormActions({
previousAction: { disabled: true, onClick: vi.fn() },
});
Expand All @@ -344,13 +347,18 @@ describe('OrganizationDetails', () => {
/>,
);

const displayNameInput = screen.getByLabelText(/display_name\.label/i);
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
expect(cancelButton).toBeDisabled();
});
});

describe('when disabled is false', () => {
it('should enable cancel button', () => {
it('should enable cancel button', async () => {
const user = userEvent.setup();
const mockFormActions = createMockFormActions({
previousAction: { disabled: false, onClick: vi.fn() },
});
Expand All @@ -361,6 +369,10 @@ describe('OrganizationDetails', () => {
/>,
);

const displayNameInput = screen.getByLabelText(/display_name\.label/i);
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
expect(cancelButton).not.toBeDisabled();
});
Expand All @@ -380,6 +392,10 @@ describe('OrganizationDetails', () => {
/>,
);

const displayNameInput = screen.getByLabelText(/display_name\.label/i);
await user.clear(displayNameInput);
await user.type(displayNameInput, 'Modified Corporation');

const cancelButton = screen.getByRole('button', { name: /cancel_button_label/i });
await user.click(cancelButton);

Expand Down
22 changes: 12 additions & 10 deletions packages/react/src/components/ui/form-actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const FormActions: React.FC<FormActionsProps> = ({
);

const showUnsavedIndicator = showUnsavedChanges && hasUnsavedChanges;
const isPreviousVisible = showUnsavedChanges ? showPrevious && hasUnsavedChanges : showPrevious;

return (
<div
Expand All @@ -79,13 +80,7 @@ export const FormActions: React.FC<FormActionsProps> = ({
)}
>
{showUnsavedIndicator && (
<div className="flex items-center gap-2">
<div
className="w-2 h-2 rounded-full bg-orange-500 transition-colors"
aria-hidden="true"
/>
<span className="text-sm text-muted-foreground">{unsavedChangesText}</span>
</div>
<span className="text-sm text-muted-foreground">{unsavedChangesText}</span>
)}

{showPrevious && (
Expand All @@ -94,8 +89,15 @@ export const FormActions: React.FC<FormActionsProps> = ({
variant={previousButtonProps.variant}
size={previousButtonProps.size}
onClick={handlePreviousClick}
disabled={previousButtonProps.disabled || isLoading}
className="FormActions-previous"
disabled={
previousButtonProps.disabled || isLoading || (showUnsavedChanges && !hasUnsavedChanges)
}
Comment on lines +92 to +94
Copy link
Contributor

@rax7389 rax7389 Jan 30, 2026

Choose a reason for hiding this comment

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

Interested to understand why we have to tweak this disabled logic?

className={cn(
'FormActions-previous',
showUnsavedChanges && !isPreviousVisible && 'invisible',
)}
aria-hidden={showUnsavedChanges && !isPreviousVisible}
tabIndex={isPreviousVisible ? 0 : -1}
>
{previousButtonProps.label}
</Button>
Expand All @@ -107,7 +109,7 @@ export const FormActions: React.FC<FormActionsProps> = ({
variant={nextButtonProps.variant}
size={nextButtonProps.size}
disabled={nextButtonProps.disabled || isLoading}
className="FormActions-next"
className="FormActions-next min-w-17.5"
{...(nextButtonProps.type !== 'submit' && { onClick: handleNextClick })}
>
{isLoading ? (
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/components/ui/wizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ function Wizard({
isLoading={isLoading}
showPrevious={showPrevious}
showNext={showNext}
showUnsavedChanges={false}
Copy link
Contributor

@rax7389 rax7389 Jan 30, 2026

Choose a reason for hiding this comment

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

any reason that we had to pass it here explicitly now?

nextAction={{
type: 'button',
label: isLastStep ? labels.completeButtonLabel : labels.nextButtonLabel,
Expand Down
Loading