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

fix(html): replace main element with div #45

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

0x-r4bbit
Copy link
Contributor

the <main> element can only be used once per page
this commit fixes it

0x-r4bbit added 2 commits May 18, 2014 11:51
the `<main>` element can only be used *once* per page
this commit fixes it
@cburgdorf
Copy link
Contributor

Hey, thanks for jumping on this. I'm super happy to see mobile support brewing. I think there are a couple of things that need to be addressed before this can be merged:

  • while the UI looks much better on mobile now, it seems to introduce some regressions for desktop. The logo seems to be too small and I'm also not a fan of stacking the input boxes vertically on desktop. While it makes sense to place them vertically for small screens, I love the unified box for larger screens.
  • please make sure that all css classes are prefixed with sw. Actually sw is kinda deprecated and we should rename everything to ny or ninya or something alike but having classes without a project prefix is a no go.
  • you seem to have thrown much stuff into one scss file. While I agree that our current structure feels a bit overly complex, I'm strongly against throwing too many things together in one file.

Otherwise, I really like it!

@0x-r4bbit
Copy link
Contributor Author

Alright, will update the PR

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