Skip to content

Conversation

@Ron-Lavi
Copy link
Collaborator

As part of the LoginPage implementation in Foreman
Adding loading spinner and disable mode to the login submit button.

@patternfly-build
Copy link
Collaborator

PatternFly-React preview: https://944-pr-patternfly-react-patternfly.surge.sh

@coveralls
Copy link

coveralls commented Nov 20, 2018

Pull Request Test Coverage Report for Build 3298

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 81.395%

Totals Coverage Status
Change from base Build 3292: 0.002%
Covered Lines: 3874
Relevant Lines: 4486

💛 - Coveralls

showError,
attributes
attributes,
isLoading
Copy link
Member

Choose a reason for hiding this comment

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

perhaps isSubmitting ?

{isLoading ? (
<span>
{' '}
<Icon name="spinner" spin size="lg" />
Copy link
Member

Choose a reason for hiding this comment

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

perhaps use the spinner component?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we have one ? :O

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a link to the PF3 react spinner (using your storybook preview url, btw)

@Ron-Lavi Ron-Lavi force-pushed the fix/login-page-on-submit branch from 15a06ea to c7c04c3 Compare November 20, 2018 12:08
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

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

the propTypes should have comments to show up in the storybook descriptions. However, since none of the pre-existing loginPage components do, you can add them or add an issue to have them added in the future.

Please add tests for the LoginCardSubmitButton.

@Ron-Lavi
Copy link
Collaborator Author

Thanks, I will polish the proptypes in another PR,
I will add test later today

@Ron-Lavi
Copy link
Collaborator Author

@jeff-phillips-18 though it seems that LoginCardSubmitButton is covered 100%:
selection_084

@jgiardino
Copy link
Contributor

I took a quick look, and don't see any issues from the html/css side.

@jeff-phillips-18 jeff-phillips-18 merged commit 769b67b into patternfly:master Nov 20, 2018
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.

8 participants