Skip to content

feat(settings): add loading state to save buttons #11639

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AMoreaux
Copy link
Contributor

Introduce a loading state to SaveButton and SaveAndCancelButtons components to enhance user feedback during save operations. Update SettingsNewObject to manage the loading state while submitting the form.

Fix twentyhq/core-team-issues#572

Introduce a loading state to SaveButton and SaveAndCancelButtons components to enhance user feedback during save operations. Update SettingsNewObject to manage the loading state while submitting the form.
@AMoreaux AMoreaux self-assigned this Apr 17, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added loading state functionality to save buttons in the settings pages to provide better user feedback during form submissions.

  • Added isLoading state management in packages/twenty-front/src/pages/settings/data-model/SettingsNewObject.tsx to handle API call states
  • Implemented isLoading prop in packages/twenty-front/src/modules/settings/components/SaveAndCancelButtons/SaveButton.tsx for visual feedback
  • Extended SaveAndCancelButtons component to propagate loading state to child components

3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

@Weiko
Copy link
Member

Weiko commented Apr 17, 2025

@AMoreaux

Screen.Recording.2025-04-17.at.18.43.46.mov

@@ -19,12 +19,13 @@ import { useNavigateSettings } from '~/hooks/useNavigateSettings';
import { getSettingsPath } from '~/utils/navigation/getSettingsPath';
import { H2Title } from 'twenty-ui/display';
import { Section } from 'twenty-ui/layout';
import { useState } from 'react';

export const SettingsNewObject = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want this for Field creation as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@AMoreaux seems the ticket says Object & Field 🙂 twentyhq/core-team-issues#572

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is unnecessary to include it during field creation, as there is no loading time; the experience is immediate.

Copy link
Member

Choose a reason for hiding this comment

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

Field takes time to appear after redirect, maybe we prefer to have a similar experience than object creation and wait for creation completion before redirection. Wdyt @charlesBochet ?

@Weiko
Copy link
Member

Weiko commented Apr 17, 2025

@AMoreaux

Screen.Recording.2025-04-17.at.18.43.46.mov

Discussed offline, we will fix it in another PR since this one is simply reusing the existing component (FYI @charlesBochet)

Replaced the tertiary font color fallback with a CSS variable (--tw-button-color) for better consistency with the design system and theming. Ensures more predictable and customizable styling.
@AMoreaux
Copy link
Contributor Author

@AMoreaux
Screen.Recording.2025-04-17.at.18.43.46.mov

Discussed offline, we will fix it in another PR since this one is simply reusing the existing component (FYI @charlesBochet)

The issue is about the space between the ellipsis and the letter S.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Loading button" on Object & Field Creation
2 participants