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

Switch to apollo #13

Merged
merged 17 commits into from
Apr 15, 2017
Merged

Switch to apollo #13

merged 17 commits into from
Apr 15, 2017

Conversation

xavxyz
Copy link
Member

@xavxyz xavxyz commented Mar 21, 2017

WIP
edit: ⬇️ see below :octocat:

@stubailo
Copy link
Member

Oooh, yarn - fancy!

@@ -12,11 +12,15 @@ export function parseMD(md) {

const contentBlocks = parseContentBlocks(content, metadata);

// these lines have already been used to create the introduction step,
// remove them from the step root
const { lineStart, lineEnd, ...cleanMetadata } = metadata;
Copy link
Member Author

Choose a reason for hiding this comment

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

in reference to #11

}

render() {
const { location, tour } = this.props;

const [tourAuthor] = tour.repository.split('/');
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm starting to simplify the components, more work to do on them later 💯 (💅?)

);

export default compose(withTour, displayLoadingState, displayNotFoundState, withMutation, pure)(
Tour
Copy link
Member Author

@xavxyz xavxyz Mar 23, 2017

Choose a reason for hiding this comment

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

prettier 🙄

It's a really cool tool, but sometimes I feel like it gets crazy (especially with gql) -> weird indentation

@@ -15,5 +15,4 @@
</head>

<body>
<div id="root"></div>
Copy link
Member Author

Choose a reason for hiding this comment

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

This one tricked me 🙄 "why the hell do I have two pages?" 😄

xavxyz added 2 commits March 24, 2017 00:17
add index to step schema;
clean stuff: collection helpers, components methods
}),
{
configServer: server => {
server.use('/graphql', OpticsAgent.middleware());
Copy link
Member Author

Choose a reason for hiding this comment

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

// basic *forced* update, we shouldn't do that normally (automatic update)
// note: tried without, it's not really updating the ui, maybe it's because
// of how we are doing our mutation?
refetchQueries: ['getTourWithSteps'],
Copy link
Member Author

Choose a reason for hiding this comment

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

💡 makes sense! we remove and then insert the tour, so the _id changes and so the store freaks out! 🌮 🎉 fixing that.

xavxyz added 2 commits March 24, 2017 15:58
* perform upserts on tour & steps when importing
* the upserts keep track of the `_id` ➡️ no need of brutal refetchQueries for `getTourWithSteps` ; also use smooth updateQueries instead of refetchQueries on Sidebar
* rename some variables & keys when parsing ➡️ more readability, clean resolvers & schema
@xavxyz xavxyz requested a review from stubailo March 24, 2017 15:03
@xavxyz
Copy link
Member Author

xavxyz commented Mar 24, 2017

🎈 Ready to take off

We have a fancy GraphQL layer handling our tours & steps 💃

Server-side capabilities

  • Queries
  • Mutations
  • Types
  • Server-side rendering with renderDataToString (including headtags \o/)
  • Wired to Optics

Data loading from the UI

  • Containers loading data over DDP replaced by containers loading data with graphql HOC: search recent tours, load a tour and its steps, load a step
  • Import tour mutation
  • Moving between steps (prev & next in the schema)

Next steps (by documenting ideas/issues)

We have two types of users: the writer ✍️ and the reader 👀 (..... who can become a 🗣!) We have room for a lot of crazy interesting stuff here! 🚀

On the ✍️ experience, we could improve how we parse files and provide more guidance / helpful errors / more automatic flow (#1) ? Also supporting images could be great (#2)
Edit: prefetching steps when hovering the button, transitions between routes / components re-loading, good way to display errors

On the 👀 experience, we could improve the navigation between steps with key controls, making sections/code easier to read (#6).

➡️ All this to lead to an awesome code documentation / presentation tool!

And in fact there is another kind of users: the contributors :party_parrot: :octocat:

When this PR is merged I'm really into adding storybook and simplify components. In this process, I'm in favor of using styled-components:

  • theming, you could theme your tour!
  • colocate your styles with your components, among the benefits: readability, real CSS with the power of JS, no more importing tons of LESS files for bootstrap (lol), no more inline-styles

I'm thinking also about:

  • batching requests server-side with dataloader for the technical experiments ;
  • add tests on the client with storyshots (jest snapshots fusions w/ storybook)
  • add tests on mongo / github connector.
  • use fragments for our queries (and maybe .graphql files?)

Yay, here are the things that comes through my head at the moment 🦆 (emoji the closest to a parrot)!

What do you think?

@xavxyz
Copy link
Member Author

xavxyz commented Apr 2, 2017

Shall I merge dat? I've already started something for the new components on simpler-components branch

@stubailo
Copy link
Member

@xavcz hey, this looks awesome - going to merge and deploy. Sorry I've been slow on this, got hit like a truck by the Apollo 1.0 release :]

@stubailo stubailo merged commit 355a35d into master Apr 15, 2017
@stubailo
Copy link
Member

@xavcz BTW I set up Optics on the partyparrot org so you should be able to see it here, looks like it's pretty fast! 🎉 https://optics.apollodata.com/service/partyparrot-codetours

@stubailo
Copy link
Member

I did have to re-import all of the tours after deploying, but it seems to work fine now :]

@xavxyz
Copy link
Member Author

xavxyz commented Apr 15, 2017

🎉🎉 that rocks!

No bother for the wait, congrats to you and the team for delivering it!! Great job 🎩

I have commit right but I'm not in the party parrot organization, can you add me please? 😁

@stubailo
Copy link
Member

Done!

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.

2 participants