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

[GH#4] Addon Panel UI #5

Closed

Conversation

brendanrygus
Copy link
Contributor

@brendanrygus brendanrygus commented Sep 16, 2020

Background

Related issue: #4

Proposed Changes

  • Create new UI components to display mocked Query response and result, leveraging Storybook's components
  • Create a new ApolloClientPanel component to interface with Story apolloClient parameters
  • Create utilities to parse the MockedProvider "mocks" prop input and convert data for display
  • Add ErrorBoundary to prevent unexpected behaviour in the addon UI from crashing the parent Storybook instance
  • Add a new section to the readme, to cover registering the panel addon

Decorator Usage

So far, I have only tested this using the addon at the Story decorator level using v3.
From my experience, the MockedProvider is only useful as a global decorator if all stories use the same mocked data. Ex. it is difficult to switch between a successfully loaded Story, and a loading state Story, when using the global decorator.

Potential UI or configuration improvements:

  • Allow users to expand/toggle the information for each Query (accordion-style), for components that have multiple mocked Queries
  • Allow consumers to option to default to the Request or Response tab. I included this in an initial pass, but also liked the existing parameters API which maps directly to the MockedProvider props.

Screenshots

SB-Addon-Query

SB-Addon-Apollo

@brendanrygus brendanrygus marked this pull request as draft September 16, 2020 17:11
@lifeiscontent lifeiscontent added the enhancement New feature or request label Sep 16, 2020
@lifeiscontent
Copy link
Owner

@brendanrygus great work. I'll have a look at this tonight, if I want to do some changes would you want me to open up a PR against your branch or just commit directly?

just from the looks of it we might be able to use some of the built in types from the graphql package. also I haven't dug into the details but I'm wondering if we should use parseAST from graphql for the query formatting.

Anyway, looking forward to digging into this more!

@brendanrygus
Copy link
Contributor Author

@lifeiscontent Oh yeah, feel free to commit directly to the branch. I don't have a lot of experience working with the graphql types/utilities aside from the basic buildSchema, so if I've reinvented the wheel somewhere let's definitely simplify things.

@lifeiscontent
Copy link
Owner

lifeiscontent commented Sep 21, 2020

@brendanrygus Hey, I took a look at this PR over the weekend. I couldn't figure out how to get it to register the panel. I think what I might end up doing is remove tsdx as the plugin itself will only need to run in the context of storybook so I'll just use tsc directly.

I tried doing this, however still couldn't get the panel UI to register.

2 things, would you want to pair on this together or try to take another stab at it with removing tsdx?

it also looks like you went off the master branch which doesn't use apollo 3.0 by default.

it's a bit of a weird setup, but in order to maintain multiple forks I have branches for each major version I think what you meant to use was to use the 3.0 branch

src/register.tsx Outdated
addons.add(PANEL_ID, {
title,
type: types.PANEL,
render: ({ active = false, key }) => (
Copy link
Owner

@lifeiscontent lifeiscontent Sep 21, 2020

Choose a reason for hiding this comment

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

@brendanrygus I think we might want to follow the same convention the storybook team uses for panels here.

https://github.com/storybookjs/storybook/blob/master/addons/knobs/src/register.tsx

rather than rendering a component tree, it's a single component if we need to change anything in the future, this kind of pattern will give us more flexibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, makes sense to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is updated now, forgot to comment last time 👍

@brendanrygus
Copy link
Contributor Author

Thanks for testing and your feedback. I got reassigned on to a project and am no longer able to contribute to OSS during my 9-5, but hopefully will have a chance to get back into things this weekend.

  1. I will update the branch to 3.x for sure.

  2. Just to make sure...had you rebuilt the plugin with the changes using TSDX before trying to pull it in on your Storybook client? I had it working using a locally built dist...but admittedly the whole process was a bit of a hack.

I'll try the tsc approach for a bit - let me know if you have a stash or branch to start with - and if I can't get anywhere asynchronously I'm down to pair on it some time too.

@brendanrygus brendanrygus changed the base branch from master to 3.x September 24, 2020 01:19
@lifeiscontent
Copy link
Owner

@brendanrygus sorry about the delay, I've got a few other things on my plate. I hope to get to this soon.

@JohannesKlauss
Copy link

@lifeiscontent any updates on this? Would really like to use it.

@brendanrygus
Copy link
Contributor Author

@JohannesKlauss Hey, sorry, I haven't had much time to dig into the build step either. Going to reach out to a TS expert I know and see if he can help with some next steps!

@lifeiscontent
Copy link
Owner

lifeiscontent commented Oct 23, 2020

I plan on working on this more next weekend. I've been busy on other projects.

@lifeiscontent
Copy link
Owner

@brendanrygus again, if you want, we can pair on it together if you have time! 👍

@brendanrygus
Copy link
Contributor Author

@lifeiscontent thanks, I hope to find the time soon 🙏
the TS wiz @micmro raised a PR against my fork that should be able to help, by building a separate register entry point (without abandoning TSDX!) brendanrygus#1

I'll test it out this week then pass it on to you

@brendanrygus
Copy link
Contributor Author

This branch is now updated with the latest build changes. Thanks a million to @micmro for taking this on during his time off from our regular job!

Could you take another shot at registering the plugin in your realworld app?
I can revisit the readme tonight or tomorrow to include the /register entry point as well

@brendanrygus brendanrygus marked this pull request as ready for review November 26, 2020 01:39
@brendanrygus
Copy link
Contributor Author

Updated the readme as well, give it a test run when you can

@lifeiscontent
Copy link
Owner

@brendanrygus @micmro super excited to see this come together! Happy holidays to the both of you. I'll try to look at it on Saturday.

Take care guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants