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.

Sign up for GitHub

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

Jump to bottom

Migrate ra-ui-materialui to TypeScript (Part 1: auth) #2984

Merged
merged 6 commits into from
Apr 17, 2019
Merged

Migrate ra-ui-materialui to TypeScript (Part 1: auth) #2984

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1c95a1d
Migrate ra-ui-materialui to TypeScript
Mar 10, 2019
5f8cf07
Review + fix warnings
djhi Mar 27, 2019
46ca99b
review 2
Mar 28, 2019
0f25f6c
Review
Apr 17, 2019
3298d42
Renaming
Apr 17, 2019
a891d9c
Fix TypeScript
Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Review
  • Loading branch information
Gildas Garcia committed Apr 17, 2019
commit 0f25f6ce43ed166207134a302c461560f47dfe5d
13 changes: 4 additions & 9 deletions packages/ra-ui-materialui/src/auth/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import defaultTheme from '../defaultTheme';
import Notification from '../layout/Notification';
import DefaultLoginForm from './LoginForm';

interface Props extends HtmlHTMLAttributes<HTMLDivElement> {
interface Props {
backgroundImage?: string;
loginForm: ReactElement<any>;
theme: object;
Expand Down Expand Up @@ -74,7 +74,9 @@ const styles = (theme: Theme) =>
* </Admin>
* );
*/
class LoginView extends Component<Props & WithStyles<typeof styles>> {
class LoginView extends Component<
Props & WithStyles<typeof styles> & HtmlHTMLAttributes<HTMLDivElement>
> {
static propTypes = {
backgroundImage: PropTypes.string,
loginForm: PropTypes.element,
Expand All @@ -91,13 +93,6 @@ class LoginView extends Component<Props & WithStyles<typeof styles>> {
containerRef = React.createRef<HTMLDivElement>();
backgroundImageLoaded = false;

// 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.
// @see https://reactjs.org/docs/refs-and-the-dom.html#adding-a-ref-to-a-dom-element
updateBackgroundImage = () => {
Copy link
Member

Choose a reason for hiding this comment

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

why did you remove the lastTry? See the comment above the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, because this code does not work at all, read the original carefully and look where we were passing lastTry. Second because TypeScript agrees with me

Copy link
Member

Choose a reason for hiding this comment

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

then you should remove the comment above the method, too

if (!this.backgroundImageLoaded && this.containerRef.current) {
const { backgroundImage } = this.props;
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/auth/Logout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
TranslationContextProps,
} from 'ra-core';

interface Props extends MenuItemProps {
interface Props {
redirectTo?: string;
}

Expand Down Expand Up @@ -45,7 +45,7 @@ const styles = (theme: Theme) =>
*
* Used for the Logout Menu item in the sidebar
*/
const LogoutView: SFC<Props & EnhancedProps> = ({
const LogoutView: SFC<Props & EnhancedProps & MenuItemProps> = ({
classes,
className,
locale,
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved
Expand Down