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

feat: add error-boundary & error-indicator components #16

Merged
merged 12 commits into from
Aug 2, 2019
Merged

feat: add error-boundary & error-indicator components #16

merged 12 commits into from
Aug 2, 2019

Conversation

Aliaksandr-Shliayeu
Copy link
Collaborator

@Aliaksandr-Shliayeu Aliaksandr-Shliayeu commented Jul 30, 2019

Error handler for "hard errors"

##Includes:

  •             NEW <ErrorBoundy /> component
    
  •            NEW < ErrorIndicator /> component
    
  •            UPDATED < App /> component
    

image

@Aliaksandr-Shliayeu Aliaksandr-Shliayeu marked this pull request as ready for review July 30, 2019 08:25
@js-machine js-machine self-requested a review July 30, 2019 14:20
@@ -3,10 +3,10 @@ import 'styles/partnersLogo.css';

export const PartnersLogo: React.FC = memo(() => {
return (
<div className="partners-logo">
Copy link
Owner

Choose a reason for hiding this comment

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

do you need this changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably not, but following the "react" best practice, empty tags should be self-closing. Also, I have deleted unnecessary spaces at the end of the string.

hasError: boolean;
}

export default class ErrorBoundry extends React.Component<{}, ErrorBoundryState> {
Copy link
Owner

Choose a reason for hiding this comment

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

don't use default export.
We don't have code style guide for now and strict rules for that,
but let's do now simple as possible :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed defaults exports from ErrorBoundry and ErrorIndicator components

@@ -25,3 +24,5 @@ export default class ErrorBoundry extends React.Component<{}, ErrorBoundryState>
return this.props.children;
}
}

export { ErrorBoundry };
Copy link
Owner

Choose a reason for hiding this comment

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

should be export class ErrorBoundry extends React.Component<{}, ErrorBoundryState> {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

import ErrorBoundry from './error-boundry';

export {ErrorBoundry};
export {ErrorBoundry} from './error-boundry';
Copy link
Owner

Choose a reason for hiding this comment

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

export * from './error-boundry'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

import ErrorIndicator from './error-indicator';

export {ErrorIndicator};
export { ErrorIndicator } from './error-indicator';
Copy link
Owner

Choose a reason for hiding this comment

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

the same as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@js-machine js-machine self-requested a review August 1, 2019 07:53
@vharadkou vharadkou changed the title feat: Added error-boundy & error-indicator components feat: add error-boundy & error-indicator components Aug 1, 2019
@vharadkou vharadkou changed the title feat: add error-boundy & error-indicator components feat: Added error-boundary & error-indicator components Aug 1, 2019
@@ -0,0 +1,26 @@
import * as React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo, rename component to error-boundary

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@vharadkou vharadkou merged commit fd410d2 into js-machine:master Aug 2, 2019
@vharadkou vharadkou changed the title feat: Added error-boundary & error-indicator components feat: add error-boundary & error-indicator components Aug 2, 2019
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