-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Render Title with context instead of React Portal #8872
Comments
I agree this would be better. We had this before contexts were a thing. Unfortunately, this would be a breaking change so it'll have to wait for v5 |
@elstgav This would be great! Would allow me to delete a lot of code which is always nice |
This suggestion poses a challenge because we allow the user to customize the title from within the view context. Let me explain with an example. Let's say the Post Edit view defines a custom title component that displays the post title. const PostEdit = () => (
<Edit title={<PostTitle />}>
...
</Edit>
); The custom title component must access the record with const PostTitle = () => {
const translate = useTranslate();
const record = useRecordContext();
return (
<>
{record ? translate('post.edit.title', { title: record.title }) : ''}
</>
);
}; Currently, this component is currently rendered by // shortened for simplicity
const Edit = ({ resource, id, title, children }) => {
const { data: record, isLoading } = useGetOne(resource, { id });
if (isLoading) return null;
return (
<RecordContextProvider value={record}>
<Title title={title} />
{children}
</RecordContextProvider>
);
}; The Now, suppose that The only solution would be to store in the TitleContext the rendered element instead of the element itself. But then how can we render an individual element, that may be composed of HTML tags ? How can we mix this with another render tree? React doesn't provide APIs for that. To update one part of the UI from another, React offers Portals, and that's why I think we already have the right implementation. Now, your issue mentions two requirements:
|
I think portals actually solve this issue. Although it will render in another part of the DOM, it still will be in the |
@djhi Yes, that's what I wanted to say. The current implementation, using portals, works. An alternative implementation based on Contexts, as demanded in this issue, can't work. So I'm closing this issue as it would be too much of a breaking change for a benefit that isn't apparent. Feel free to add comments if you have use cases that require that change. |
Currently the
<Title>
is rendered to a React portal; either<TitlePortal/>
or any component with thereact-admin-title
ID. This approach is fairly brittle, as you can only render the title once on the page, and there’s no way to access the current page title. This also makes it difficult to implement dynamic window titles (like in #5893)Describe the solution you’d like:
Instead of
<Title>
rendering with a React portal, have it set the title in the globalCoreAdminContext
instead, and provide a helper to access the title, e.g.useTitle() // => { title, defaultTitle, ...etc }
.This would greatly simplify rendering the title in multiple/other places, and could simplify updating the window title with something like React Helmet:
Additional context
I think this would simplify:
The text was updated successfully, but these errors were encountered: