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

Basic desktop CSS #70

Merged
merged 5 commits into from Jan 4, 2019
Merged

Basic desktop CSS #70

merged 5 commits into from Jan 4, 2019

Conversation

roelofjan-elsinga
Copy link
Contributor

I've added some basic CSS rules to scale the mobile version better on the desktop. The homepage is quite literally just the mobile view with a max-width and horizontally aligned.

homepage plant app

On the plant detail page the tags will stack on desktop views, and keep the horizontal scroll on the mobile view.

desktop plant page

Editing the tags looks very similar to before, just made sure it scaled well from mobile to the desktop version. I've added a margin-top on the editing field, because I want to make better use of the newly created space on the desktop view.

desktop editing tags

@morkro
Copy link
Owner

morkro commented Jan 2, 2019

Hey @roelofjan-elsinga, thank you for your contribution!

Coincidentally, I have just updated various screens on desktop in the open #69 Release 1.6.0 pull request as well. :) The Plant screen got some updates for desktop as well (basically a two column layout) and your changes there will be overwritten.

Just a few comments. I see you've added the same media query quite a bit.

@media screen and (min-width: 768px) {

Can you create a @custom-media for it in src/styles/media-queries.css? Might actually just update (and rename?) --max-mobile-viewport and use that one since it hasn't been used by now.

The 16px margins can be replaced with var(--base-gap).

@roelofjan-elsinga
Copy link
Contributor Author

@morkro I'll implement those changes later today, thanks for the clear comment 😃

@roelofjan-elsinga
Copy link
Contributor Author

@morkro I've kept the 600px you had defined under --max-mobile-viewport, because it didn't make a huge difference for my layouts.

@morkro morkro changed the base branch from master to release/1.6.0 January 4, 2019 13:12
@morkro morkro merged commit 91f68c7 into morkro:release/1.6.0 Jan 4, 2019
@morkro
Copy link
Owner

morkro commented Jan 4, 2019

Thanks @roelofjan-elsinga — I've updated a few minor things to fit my other desktop changes for the Plant screen :)

@roelofjan-elsinga
Copy link
Contributor Author

Perfect @morkro! I'll look for some more things I can help with :)

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