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

Move Docs to Vercel #652

Merged
merged 6 commits into from Aug 5, 2020
Merged

Move Docs to Vercel #652

merged 6 commits into from Aug 5, 2020

Conversation

mcsdevv
Copy link
Contributor

@mcsdevv mcsdevv commented Jun 29, 2020

This PR gets rid of server.js and routes.js in favor of Next.js redirects.

It also makes use of the Router and Link components provided by Next.js.

There may be some behavior with the routing that has been changed and requires amending. If so, please let me know in the comments here and we can do so.

Edit: definitely scope for another PR upgrading Next.js and making the docs fully static!

@mathisonian
Copy link
Member

Thanks @mcsdevv!! This is huge, testing it out now I'll see if I notice anything funny with the routing.

@mathisonian
Copy link
Member

Okay things look close but I'm seeing a few issues:

  1. Navigating to the gallery page (https://idyll-lang.org/gallery) gives an error (see screenshot below)

Screen Shot 2020-06-30 at 4 22 18 PM

  1. The editor seems to break when the page is saved. To reproduce go to the editor page and hit the fullscreen button. It may take a few seconds to respond but then you'll see an error in the JS console related to Router.push is not a function.

Screen Shot 2020-06-30 at 4 25 06 PM

Any insight you have on these issues is appreciated.

@mcsdevv
Copy link
Contributor Author

mcsdevv commented Jun 30, 2020

Hey @mathisonian, I've got some ideas and will take a look tomorrow 👍

@mathisonian
Copy link
Member

Sounds good - thanks @mcsdevv!

@mcsdevv
Copy link
Contributor Author

mcsdevv commented Jul 1, 2020

@mathisonian a couple of questions, I'm not overly familiar with this version of Next.js so please excuse:

  • There's a hrefFromName method being imported into next.config.js from the root contents.js file but no export, can you explain this to me?
  • Is it expected that the save functionality for the editor should work in development also? (I fixed the router import but this could be an incorrect redirect I added)

@mathisonian
Copy link
Member

@mcsdevv I think it is safe to ignore / remove that next.config.js it doesn't seem to be being used by anything, and would be incorrect anyways (as you point out, hrefFromName) is undefined. These docs have evolved over time from a number of different contributors so there are a few cobwebs.

With respect to save functionality in development, yes I would expect that to work (it does currently if you run the version on master). The saving functionality is handled via another app which exposes API so there isn't much logic in these docs other than sending a request, waiting for a response, and redirecting to the correct spot based on the response.

@mcsdevv
Copy link
Contributor Author

mcsdevv commented Jul 7, 2020

Hey @mathisonian, thanks for confirming. I'm struggling to find the time to finish this right now, I think it's all good barring the save functionality. I'll try and get to it next week.

@mathisonian
Copy link
Member

Thanks @mcsdevv!

@mathisonian
Copy link
Member

Hey @mcsdevv, any chance you think you'll have some time to look at this this week before Vercel cuts of the 1.0 deployments?

@mathisonian
Copy link
Member

I'm going to merge this now, because partially broken docs are better than deleted docs :/

We'll have to fix the rest later!

@mathisonian mathisonian merged commit 19e4568 into idyll-lang:master Aug 5, 2020
@mcsdevv
Copy link
Contributor Author

mcsdevv commented Aug 5, 2020

Hey @mathisonian, sorry I've been a little busy lately, I'll ask a member of the team to take a look though!

@mathisonian
Copy link
Member

Thanks @mcsdevv! Really appreciate you getting us this far

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.

None yet

2 participants