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

Rewrite style.css for better mobile experience. #5

Merged
merged 3 commits into from
Jan 2, 2023
Merged

Rewrite style.css for better mobile experience. #5

merged 3 commits into from
Jan 2, 2023

Conversation

skippyr
Copy link
Contributor

@skippyr skippyr commented Jan 2, 2023

Hello, @burntcarrot. I have rewritten the file style.css and also had to adapt the index.html . Here is some of the changes I have done:

Changes to style.css

  • add variables for colors, borders and border-radius.
  • change colors format from hex to hsl.
  • add some utils classes.
  • change layout to grid instead of flex.
  • add object-fit: cover to all images.
  • control width of elements using @media.
  • add grid to sliders, making those side by side, reducing vertical height.
  • make the yellow colors a bit darker.

Changes to index.html

  • add meta tag to control the mobile viewport.

    <meta name="viewport" content="width=device-width, initial-scale=1.0">
  • remove unecessary imports of Noto Sans. Those are already imported in the style.css.

  • add an anchor with the hyperlink of your repository to reduce vertical height.

  • make all the images not draggable.

  • correct punctuation of textarea's placeholder.

Remaining issues

  • The font for the text that is above the lake can be small in mobile devices. That is something that might be fixed by changing the speak.js, but I preferred not to do it in this PR.
  • The sliders' thumb does not change color on Firefox because it does not offer support to -webkit-slider-thumb.

I hope this fix the issue #1.

Sherman Rofeman added 2 commits January 2, 2023 13:40
Now, whenever the screen goes too small in width, the layout goes vertical.
@vercel
Copy link

vercel bot commented Jan 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
quackspeak ✅ Ready (Inspect) Visit Preview Jan 2, 2023 at 6:21PM (UTC)

@burntcarrot
Copy link
Owner

Thank you for your work on this PR!

Here are a few things I wanted to ask:

change colors format from hex to hsl.

Is there any advantage of using hsl over hex?

The font for the text that is above the lake can be small in mobile devices. That is something that might be fixed by changing the speak.js, but I preferred not to do it in this PR.

Do you want me to create a new issue for this?

Related to this, the header above the lake is large in mobile devices; could you please fix this please? Currently, the header adds no value in mobile devices.

@skippyr
Copy link
Contributor Author

skippyr commented Jan 2, 2023

About hsl, they are easier to understand than hex and for this reason they are often used.

About the header, I will check a way to make it smaller, or do you want to remove it in mobile devices?

@burntcarrot
Copy link
Owner

do you want to remove it in mobile devices?

Definitely. Let's remove it in mobile devices - it takes up a lot of real estate and doesn't add any value.

Can you make the changes in this PR itself, considering that this PR deals with mobile devices?

@skippyr
Copy link
Contributor Author

skippyr commented Jan 2, 2023

Sure.

Now, the heading is omitted whenever the screen can not hover, which means it is a mobile device.
@skippyr
Copy link
Contributor Author

skippyr commented Jan 2, 2023

I have added a new media query that omits the heading whenever the screen does not support hover, which means we are dealing with mobile devices.

@skippyr
Copy link
Contributor Author

skippyr commented Jan 2, 2023

About the font size above the lake, it is better to leave that to another issue.

@burntcarrot
Copy link
Owner

I have added a new media query that omits the heading whenever the screen does not support hover, which means we are dealing with mobile devices.

Thanks for using media queries! Makes it much more maintainable.

About the font size above the lake, it is better to leave that to another issue.

Great. I'll create a new issue for it.

Copy link
Owner

@burntcarrot burntcarrot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff on this PR! Thanks for your efforts!

Merging.

@burntcarrot burntcarrot merged commit 4cef862 into burntcarrot:main Jan 2, 2023
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