edit / show title prop: add 'false' and callback choices #9770
Conversation
hiaselhans
changed the title
There was a problem hiding this comment.
Thank you for your contribution 😊
Two more things I'd like to add to the following comments:
- I saw the following warning in the console while testing:
Warning: Failed prop type: Invalid prop `title` supplied to `Show`, expected a ReactNode.. You may have forgotten to update a propTypes declaration somewhere. - I'd like to have the approval of @fzaninotto or @djhi on the feature before merging. Guys wdyt?
| @@ -71,7 +71,9 @@ That's enough to display the post show view above. | |||
| | `queryOptions` | Optional | `object` | | The options to pass to the `useQuery` hook | |||
| | `resource` | Optional | `string` | | The resource name, e.g. `posts` | |||
| | `sx` | Optional | `object` | | Override or extend the styles applied to the component | |||
| | `title` | Optional | `string | ReactElement` | | The title to display in the App Bar | |||
| | `title` | Optional | *titleType | | The title to display in the App Bar see [Edit](./Edit.md#title) | |||
There was a problem hiding this comment.
nitpick (non-blocking): You could also add another example in the title section of this doc, just like you did for Edit.
| <Title | ||
| title={newTitle} | ||
| defaultTitle={defaultTitle} | ||
| preferenceKey={`${resource}.show.title`} |
There was a problem hiding this comment.
issue:
| preferenceKey={`${resource}.show.title`} | |
| preferenceKey={`${resource}.edit.title`} |
| it('should display a custom title', async () => { | ||
| render(<CustomTitle />); | ||
| await screen.findByText('book_1_test'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
question: Why not add tests for Edit too?
| export type ShowViewProps<RecordType extends RaRecord = any> = ShowProps< | ||
| RecordType | ||
| >; |
There was a problem hiding this comment.
question: Why did you need that change?
| | string | ||
| | ReactElement | ||
| | false | ||
| | ((arg0: RecordType | undefined) => string); |
There was a problem hiding this comment.
polish:
| | ((arg0: RecordType | undefined) => string); | |
| | ((record: RecordType | undefined) => string); |
| | string | ||
| | ReactElement | ||
| | false | ||
| | ((arg0: RecordType | undefined) => string); |
There was a problem hiding this comment.
polish:
| | ((arg0: RecordType | undefined) => string); | |
| | ((record: RecordType | undefined) => string); |
|
Got the approval! 🎉 Also, realizing this will also fix #9695 |
|
Any updates on this one? |
|
Bump, you need to rebase and take the review into account. |
|
No news for some time, closing. Superseded by #9969. |
follow-up from #9768 to change branch
This PR adds two more choices to the 'title' prop of edit / show views:
I found this to be the most flexible and efficient way to inject a title