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

Experimental React-fication #42

Draft
wants to merge 40 commits into
base: main
Choose a base branch
from
Draft

Experimental React-fication #42

wants to merge 40 commits into from

Conversation

40thieves
Copy link
Collaborator

@40thieves 40thieves commented Mar 2, 2022

Work towards #33

Description

Experimental and somewhat unpolished attempt to migrate the code to React.

The main things I'd like feedback on is the pattern for exposing exercises in Exercises/*/index.js. Inspired by Remix, I've used named exports to provide an "API"

The styles & exercise content are completely not final - just enough to get it working. The Blocks folder contains stuff just copy/pasted from the root (which is probably not sensible in the long term)

How to run

$ cd client
$ npm i
$ npm start

@CodeYourFuture/syllabus-team


View rendered client/src/Exercises/01-stuff/lesson.md
View rendered client/src/Exercises/02-more-stuff/lesson.md

@netlify
Copy link

netlify bot commented Mar 2, 2022

Deploy Preview for cyf-blocks ready!

Name Link
🔨 Latest commit bfdaec9
🔍 Latest deploy log https://app.netlify.com/sites/cyf-blocks/deploys/626af196bd00a400092e8027
😎 Deploy Preview https://deploy-preview-42--cyf-blocks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@gregdyke
Copy link
Collaborator

gregdyke commented Mar 2, 2022

I had in mind traversing a folder or set of folders rather than enumerating the lessons as explicit imports with an array. But on second blush, the array seems fine

I'm not doing fancy layouts by hand...
moving to a new branch now to do layout
remove comments for trainees
make modifier consistent on double dash --
@SallyMcGrath
Copy link
Member

I was going to say what about listing them in frontmatter https://github.com/remarkjs/remark-frontmatter

But on reflection, is it that much more readable?

will also update this theme to the modern one asap
I guess really App.js should just show the base structure?
may as well do it now and stop the mess
is this how buttons work in React? Let's find out...
and really just pushing blocks around -- this is NOT a design!
darned muscle memory
sorry... I will clean this up
it's not a finished design, deliberately done in a wireframey style so we can consider UX
- put in a Split grid 
- componentised everything roughly so it's readable/navigable
- put in a wireframey style so we can play around with placement/toolbars/interactions
48px touch target minimum
stops it disappearing - I guess this is to do with the re-injection thing mentioned in useBlockly
From Blush, royalty free
just fiddling, nothing to see here
@40thieves
Copy link
Collaborator Author

I had in mind traversing a folder or set of folders rather than enumerating the lessons as explicit imports with an array

Yeah, I had that idea too, however unfortunately since we're in CRA we're limited in how we read the filesystem (i.e. we don't have access to fs) so I'm somewhat working around that. I do have some ideas regarding this but I think they should wait until later.


We also discussed in the syllabus meeting that we were going to treat the react branch (i.e. the branch behind this PR) as an integration branch --- we'll merge all other React-based work onto this branch until we're ready to flip main over to the React implementation.

I guess we didn't discuss what to do with this PR until then. I suppose I could close it until we're ready to do this flip? Or just leave it open, and let the changes accumulate?

SallyMcGrath and others added 10 commits March 17, 2022 11:04
Stacked PR (Layout dep on SASS) for Ali - just going to merge this - feel free to toss it out if you hate it
Since we're not targeting main and it's nested within the client folder, I don't think this would have worked. Instead we can just assume that prettier is set up in people's editors
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.

3 participants