-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Feature/mobile examples #1528
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
Feature/mobile examples #1528
Conversation
…com/ghalestrilo/p5.js-web-editor into feature/mobile-examples
This PR implements a bug where the navigation from
|
server/routes/server.routes.js
Outdated
|
||
// Mobile Routes | ||
if (process.env.MOBILE_ENABLED) { | ||
router.get('/mobile', (req, res) => res.send(renderIndex())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these routes are the same as the desktop routes above, one way to avoid duplication would be to mount them again under the /mobile
namespace in server.js:
// this is supposed to be TEMPORARY -- until i figure out
// isomorphic rendering
app.use('/', serverRoutes);
if (process.env.MOBILE_ENABLED) {
app.use('/mobile’, serverRoutes);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow this is amazing, I didn't know this was possible, thanks andrew!
The navigation bug has been fixed. The navigation was being prevented because |
The other changes you made are perfect 🌈 |
Great! I managed to get the header the way you suggested by fiddling with the grid layout. About the 12px left-padding: I think it's not what caused the misalignment - I think it was the cell's If the problem persists on your device, can I ask you to take a look into it? I tested a couple different resolutions and browsers here but cannot reproduce |
This is super close to being done! Just a couple more things. I made a couple of CSS adjustments, and then also:
|
I made the adjustments you suggested to better fit text on the header. I think this is ready to merge :) |
…ilo/p5.js-web-editor into feature/mobile-examples
Requires #1513
Creates mobile My Stuff / Examples views, through the
<MobileDashboardView />
I have verified that this pull request:
npm run lint
)Fixes #123