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

Homepage: visual switch for web applications section on mobile #297

Merged
merged 7 commits into from
Nov 18, 2017
Merged

Homepage: visual switch for web applications section on mobile #297

merged 7 commits into from
Nov 18, 2017

Conversation

geoffreydhuyvetters
Copy link
Contributor

@geoffreydhuyvetters geoffreydhuyvetters commented Nov 16, 2017

What kind of change does this PR introduce?

homepage change

changed position of Tailored for web applications intro as discussed here https://twitter.com/CompuIves/status/931253159871295488

@geoffreydhuyvetters geoffreydhuyvetters changed the title Switch intro for mobile screen size (WIP) Switch intro for mobile screen size Nov 16, 2017
@geoffreydhuyvetters
Copy link
Contributor Author

Fixed it this way, do you have a suggestion so we don't need to change text twice.
Should I create a new component (maybe ) in /Frameworks so we can change the text once?

Let me know and I'll do the changes + the all contributors step.

@CompuIves
Copy link
Member

Super! Hmm yes, I can think of 2 approaches we can do here:

  1. Wrap the two elements in a display: flex block. We can then do flex-direction: column-reverse for mobile and flex-direction: column; for higher. This feels like the best solution, but I'm not sure if it's possible with the current layout.
  2. Extract the selection to a component and use the Media like you do here.

Would be great to have option 1, but only if it makes sense with the current layout. Otherwise 2 is the best option, as it's very clear.

Thanks a lot for the contribution! Great to have a new contributor for CodeSandbox 😄

@geoffreydhuyvetters
Copy link
Contributor Author

I also agree 1 would be the best option, taking a look how much impact it has.

@geoffreydhuyvetters
Copy link
Contributor Author

@CompuIves Think I nailed it. 🎉

@CompuIves
Copy link
Member

That's great!! I can merge it in tomorrow, when I'm back at my MacBook. Thanks a lot!

@geoffreydhuyvetters
Copy link
Contributor Author

Sorry about the multiple contributor commits.

I thought it didn't work, but apparently it did :)

@geoffreydhuyvetters geoffreydhuyvetters changed the title (WIP) Switch intro for mobile screen size Switch intro for mobile screen size Nov 16, 2017
@geoffreydhuyvetters geoffreydhuyvetters changed the title Switch intro for mobile screen size Visual switch for web applications section on mobile Nov 16, 2017
@geoffreydhuyvetters geoffreydhuyvetters changed the title Visual switch for web applications section on mobile Homepage: visual switch for web applications section on mobile Nov 16, 2017
@CompuIves
Copy link
Member

This is great, @duivvv. Thanks! Merging it in now, welcome to the contributors 😄 .

@CompuIves CompuIves merged commit 0b861fe into codesandbox:master Nov 18, 2017
@geoffreydhuyvetters
Copy link
Contributor Author

No problem, glad I could help

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