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

[RFR] Login Background Image: handle empty ref #2596

Merged
merged 2 commits into from
Nov 30, 2018
Merged

Conversation

Kmaschta
Copy link
Contributor

Even though the React doc ensure the ref creation is done before the
componentDidMount, it can happen that the ref is set to null until the
next render.

So, to handle this case the component will now try to load the image on
the componentDidMount, but if the ref doesn't exist, it will try again
on the following componentDidUpdate. The try will be done only once.

Refs:

@Kmaschta Kmaschta force-pushed the login-background-image branch from 1b16db9 to d647073 Compare November 30, 2018 14:29
@Kmaschta Kmaschta changed the title [WIP] Login Background Image: handle empty ref [RFR] Login Background Image: handle empty ref Nov 30, 2018
@Kmaschta Kmaschta added this to the 2.5.1 milestone Nov 30, 2018
@Kmaschta Kmaschta force-pushed the login-background-image branch from 8c53392 to a8ce961 Compare November 30, 2018 14:40
Even though the React doc ensure the ref creation is done before the
componentDidMount, it can happen that the ref is set to null until the
next render.

So, to handle this case the component will now try to load the image on
the componentDidMount, but if the ref doesn't exist, it will try again
on the following componentDidUpdate. The try will be done only once.

Refs:
- https://reactjs.org/docs/refs-and-the-dom.html
@Kmaschta Kmaschta force-pushed the login-background-image branch from a8ce961 to d483c2c Compare November 30, 2018 14:41
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.

As discussed, let's avoid adding props on the Admin component. We should better document how to disable it using a custom Login page component

@Kmaschta Kmaschta force-pushed the login-background-image branch from 8d57b19 to 047114a Compare November 30, 2018 15:08
@Kmaschta Kmaschta force-pushed the login-background-image branch from 047114a to d108e0b Compare November 30, 2018 15:09
@Kmaschta
Copy link
Contributor Author

Kmaschta commented Nov 30, 2018

Good point 👍
I added a doc in the theming too, but the section of the login page seems pretty empty.

If you want, I can add more points to the Using a Custom Login Page with more detailed examples (from Stack Overflow for example)

@djhi djhi merged commit da1b84c into master Nov 30, 2018
@djhi djhi deleted the login-background-image branch November 30, 2018 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants