Skip to content

test: add ErrorModal tests and story #2325

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

Merged
merged 6 commits into from
Jul 30, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
docs(ErrorModal): add stories
  • Loading branch information
Alexis committed Jul 25, 2023
commit 0428ee559df079b0290c15d7322fbec338c7464c
47 changes: 47 additions & 0 deletions client/modules/IDE/components/ErrorModal.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from 'react';

import ErrorModal from './ErrorModal';

export default {
title: 'IDE/ErrorModal',
component: ErrorModal,
argTypes: {
type: {
options: [
'forceAuthentication',
'staleSession',
'staleProject',
'oauthError'
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might be better to define these options in the propTypes of the component rather than here. If we do it that way then Storybook will pick up on the options automatically and we'll also get the validation in prop-types.

ErrorModal.propTypes = {
  type: PropTypes.oneOf([
    'forceAuthentication',
    'staleSession',
    'staleProject',
    'oauthError'
  ]).isRequired,
  closeModal: PropTypes.func.isRequired,
  service: PropTypes.oneOf(['google', 'github'])
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The action wasn't picked up with this 😞 so left that argType.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah when I was playing around with it I left the closeModal in. I'm not aware of a trick for those, at least not yet.

FYI, if you want to keep it as a dropdown rather than radio buttons (I don't think it matters?) then you can provide a partial config like service: { control: { type: 'select' } } which specifies the control and it will still infer the options.

control: { type: 'select' }
},
service: {
options: ['google', 'github'],
control: { type: 'select' }
},
closeModal: { action: 'closed' }
}
};

const Template = (args) => <ErrorModal {...args} />;
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI I tried seeing if wrapping in <Overlay> here would show us the actual UI but does not ☹️.
image

Not a problem. Maybe something we can play with later if we want to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting. I'm still familiarising myself with the components in the application. I was wondering where the overlay was 😄 It would make a nicer story seeing the modal represented with the overlay.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to try it again after the theming changes in #2326. I think the way that they SCSS gets generated is that it creates selectors like .light .overlay and .dark .overlay but not just .overlay. So it's possible that the overlay UI will work once it's inside the right parent container.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Honestly it feels like a bit of a naming mistake that this is called ErrorModal when it doesn't include the modal part. Really it's more of an ErrorMessage!

It could potentially be modified so that it takes the overlay's title and ariaLabel as props and renders both the Overlay and the message.


export const ForceAuthenticationErrorModal = Template.bind({});
ForceAuthenticationErrorModal.args = {
type: 'forceAuthentication'
};

export const StaleSessionErrorModal = Template.bind({});
StaleSessionErrorModal.args = {
type: 'staleSession'
};

export const StaleProjectErrorModal = Template.bind({});
StaleProjectErrorModal.args = {
type: 'staleProject'
};

export const OauthErrorModal = Template.bind({});
OauthErrorModal.args = {
type: 'oauthError',
service: 'google'
};