Skip to content

Conversation

@SushilMallRC
Copy link
Contributor

No description provided.

@SushilMallRC SushilMallRC requested a review from embbnux April 22, 2024 19:07
@coveralls
Copy link

coveralls commented Apr 22, 2024

Coverage Status

coverage: 93.808% (+0.4%) from 93.418%
when pulling e16ec52 on comments
into 9e1a2d9 on master.


import {SDK} from '@ringcentral/sdk';

// Utility function to get display name
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a display name? Who's name? A user? A service? Or something else

// Utility function for delaying execution
const delay = () => new Promise(res => setTimeout(res, 0));

// Interface for the state of AuthGate component
Copy link
Contributor

Choose a reason for hiding this comment

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

Need param description

authError: null | Error;
}

// Interface for the render props provided by AuthGate
Copy link
Contributor

Choose a reason for hiding this comment

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

Need param description

logout: () => Promise<any>;
}

// Interface for the props of AuthGate component
Copy link
Contributor

Choose a reason for hiding this comment

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

Need param description

children: (props: AuthGateRenderProps) => any;
}

// AuthGate component definition
Copy link
Contributor

Choose a reason for hiding this comment

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

What does AuthGate do in general?

// Handler for before event
public before = () => this.mounted && this.setState({authorizing: true});

// Handler for error event
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do in the handler?

// Handler for error event
public error = async e => this.updateState(e);

// Handler for success event
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do in the handler?

// Handler for success event
public success = async () => this.updateState(null);

// Method to get login URL
Copy link
Contributor

Choose a reason for hiding this comment

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

What's in options?

}
};

// Method to update state
Copy link
Contributor

Choose a reason for hiding this comment

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

Need more description of this method. This looks a bit like an unauth, which is confusing. Why is authorizing set to false?

});
};

// Render method
Copy link
Contributor

Choose a reason for hiding this comment

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

Not this one, but the one below if you expand the hidden code for Line 153 ~162.

There's a clear comment saying TODO Definition there. That's a very clear sign that we want to add its definition there.

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.

3 participants