Skip to content
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

Add success notifications override per resource #10203

Merged
merged 12 commits into from
Sep 18, 2024
Merged

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Sep 13, 2024

Problem

The snackbar that confirms the creation/update/deletion use a generic "element" name. It's impersonal and sometimes confusing.

Capture d’écran 2024-09-12 à 15 54 20
Capture d’écran 2024-09-12 à 15 54 12
Capture d’écran 2024-09-12 à 15 53 54
Capture d’écran 2024-09-12 à 15 54 01

Solution

Use a per-resource notification message, and fall back to the previous message.

The per-resource messages are:

  • resources.[resource].notifications.created
  • resources.[resource].notifications.updated
  • resources.[resource].notifications.deleted

The default notification remains the same, however developers can override it using the translation messages, e.g.

const customEnglishMessages: TranslationMessages = {
    ...englishMessages,
    resources: {
        customers: {
            name: 'Customer |||| Customers',
            // ...
            notifications: {
                created: 'Customer created |||| %{smart_count} customers created',
                updated: 'Customer updated |||| %{smart_count} customers updated',
                deleted: 'Customer deleted |||| %{smart_count} customers deleted',
            },
        },
    },
};

Capture d’écran 2024-09-12 à 15 52 27
Capture d’écran 2024-09-12 à 15 52 12
Capture d’écran 2024-09-12 à 15 47 48
Capture d’écran 2024-09-12 à 15 48 08

French:

Capture d’écran 2024-09-12 à 15 53 18
Capture d’écran 2024-09-12 à 15 53 09
Capture d’écran 2024-09-12 à 15 52 48
Capture d’écran 2024-09-12 à 15 52 56

Supersedes #10201

How To Test

  1. Launch the simple example
  2. Edit a post, and save => see update one notification
  3. Edit a post, and delete => delete one notification
  4. Select several posts, and delete => delete many notification
  5. Create a new post and save => create notification

To do

  • Add per-resource notification message for create, update, and delete confirmation
  • Fix unit tests
  • Add new stories for this use case
  • Add new tests for this use case
  • Update the documentation

These buttons allow users to navigate between the various react-admin views.

### `<EditButton>`
- **Navigation Buttons**: to navigate between the various react-admin views.
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the sub sections ('navigation', 'list', etc) from the content and added this TOC instead to move all the section levels up (####=> ###). This makes the right menu much more useful.


| Rule name | Description |
|-----------------------------------------------|-------------------------------------------------|
| `&.RaSkipNavigationButton-skipToContentButton` | Applied to the underlying `MuiButton` component |

To override the style of all instances of `<SkipNavigationButton>` using the [application-wide style overrides](./AppTheme.md#theming-individual-components), use the `RaSkipNavigationButton` key.

### `<MenuItemLink>`
Copy link
Member Author

Choose a reason for hiding this comment

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

I rmeoved this one as we now document it in <Menu.Item>

@@ -163,6 +163,12 @@ To override the style of `<Menu>` using the [application-wide style overrides](.

## `<Menu.Item>`

<video controls autoplay playsinline muted loop>
Copy link
Member Author

Choose a reason for hiding this comment

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

moved from `Buttons.md#menuitemlink

## Record Buttons

### `<UpdateButton>`
## `<UpdateButton>`
Copy link
Member Author

Choose a reason for hiding this comment

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

There was a short doc for UpdateButton here, and a dedicated page in UpdateButton.md. I don't see why that button deserves a specific page. So I moved the content back here, changed the link in Reference.md to point here, and removed the UpdateButton item in the main navigation.

We must keep the UpdateButton.md file though, to avoid 404 on external links.

@@ -25,14 +25,14 @@ const AcceptButton = () => {
{
mutationMode: 'undoable',
onSuccess: () => {
notify('resources.reviews.notification.approved_success', {
notify('resources.reviews.notifications.approved_success', {
Copy link
Member Author

Choose a reason for hiding this comment

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

As the new namespace for custom resource notifications is resources.{resource}.notifications, it made sense to use the same namespace in this demo.

@fzaninotto fzaninotto added RFR Ready For Review and removed WIP Work In Progress labels Sep 17, 2024
@fzaninotto fzaninotto changed the title Add ability to override success notifications per resource Add success notifications override per resource Sep 17, 2024
Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

I know this PR is already long but I feel like we're missing a lot of stories.

| `translateOptions` Optional | `{ id?: string, name?: string }` | {} | Custom id and name to be used in the confirm dialog's title |
| `mutationOptions` Optional | | null | options for react-query `useMutation` hook |
| `successMessage` Optional | `string` | 'ra.notification.deleted' | Lets you customize the success notification message. |
| Prop | Required | Type | Default | Description |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we either repeat the DeleteButton details for props or mention they are the same?


const theme = createTheme();

export const Default = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have more stories for at least the added props

@djhi djhi merged commit d04e6d9 into next Sep 18, 2024
14 checks passed
@djhi djhi deleted the notification-resource-2 branch September 18, 2024 13:24
@djhi djhi added this to the 5.3.0 milestone Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants