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.

Sign up for GitHub

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

Jump to bottom

Markup review #60

Closed
paulirish opened this issue Jun 12, 2014 · 11 comments
Closed

Markup review #60

paulirish opened this issue Jun 12, 2014 · 11 comments

Comments

@paulirish
Copy link
Member

Update: many of these have now been addressed.

Having gone through the release of h5bp and its lifetime, I think the current state of the markup in w-s-k is too heavy. There are some things in here that are low value and unnecessary.. but some higher value things that I think could be considered to be a non-default as they just add to perception that this is too heavy and bloated. We can avoid that, while still delivering a lot of value with the project.

I'm sorry that I dumped a bunch of issues into a single ticket. :(

Details below.

       <meta name="HandheldFriendly" content="True">
        <meta name="MobileOptimized" content="320">

These are in for barely legitimate reasons. It's time for them to die.

<meta http-equiv="cleartype" content="on">

cleartype

cleartype on for mobile IE? Lets let mobile IE handle its own type rendering. This is the sort of feature that will be considered bloat.

#### touch-icons, favicon

h5bp killed the markup reference for these.
h5bp/html5-boilerplate#1367
It makes the markup feel wayyy better. let's copy that.

   <!-- Tile icon for Win8 (144x144 + tile color) -->
        <meta name="msapplication-TileImage" content="images/touch/ms-touch-icon-144x144-precomposed.png">
        <meta name="msapplication-TileColor" content="#3372DF">

This also feels low-value. I would prefer it's in https://github.com/h5bp/html5-boilerplate/blob/master/doc/extend.md and we say, consider whats in there if you want.

   <!-- Disable Tap Highlight for Windows Devices -->
        <meta name="msapplication-tap-highlight" content="no">

why disable?

#### prevents links from opening in Mobile Safari.

seems risky.

#### microdata

does it really make sense to declare every page built with this as a Article?
and meanwhile also default to being a standalone app?

 <meta name="created-with" content="Web Starter Kit">

This isn't valid, nor registered. http://wiki.whatwg.org/wiki/MetaExtensions


<!-- SEO: If mobile URL is different from desktop URL, add a canonical link to the desktop page -->
        <!--
        <link rel="canonical" href="http://www.example.com/" >
        -->

I swear that google recommends a different pattern for defining the mobile site vs desktop site. It's in Smus's multidevice h5r article from a while ago.

#### styles difference

what's the difference between what's in styles.scss vs main.css ?

<html class="no-js no-touch"

are no-js and no-touch removed somewhere?
@passy
Copy link
Collaborator

passy commented Jun 12, 2014

Agreed to all individual points as well as the general sentiment that too much markup is going to scare people away and defeats the point of being a lean starting point. I also looked up the created-by meta tag without finding anything.

The no-js no-touch classes are actually never touched by any code.

@sindresorhus
Copy link
Contributor

👍 Agree with all this. I too was wondering about these things.

passy added a commit that referenced this issue Jun 13, 2014
@passy
Clean up markup (re #60)
1c5f7d8
@passy passy mentioned this issue Jun 13, 2014
Merged
passy added a commit that referenced this issue Jun 13, 2014
@passy
Clean up markup (re #60)
54e7ead
sindresorhus added a commit that referenced this issue Jun 13, 2014
@sindresorhus
Merge pull request #61 from google/markup-cleanup
78d8a6a 
Clean up markup (re #60)
@addyosmani
Copy link
Contributor

Thanks for the feedback @paulirish. I'll put together some PRs to address these. Looks like @passy has already nailed some of them.

As no-js and no-touch aren't touched by any code, should we just remove them?

@addyosmani addyosmani mentioned this issue Jun 13, 2014
Merged
@addyosmani
Copy link
Contributor

what's the difference between what's in styles.scss vs main.css ?

styles.scss (/sass) contain the visual styles we give you from the Style Guide.

They contain the styling for buttons, grids, typography and so on. A pre-built unminified version of this file is included in the pack for those that want to use those styles, but don't opt to use the tooling.

main.css contains styles specific to the default layout included in the main boilerplate (index.html). They're extremely lean.

The reason these files are separate is to enable choice. If you don't choose to use our style guide and styles, you can simply delete them. You can still go and use index.html and main.css to get the default look and feel.

If however you're happy to go all in, you would use styles + main. Does that make sense?

Totally open to simplifying this if it's too complex

@paulirish
Copy link
Member Author

Sounds good.

Maybe "styles" to "component_styles" ?

Kind of small fish though. Im OK either way.

@sindresorhus
Copy link
Contributor

How about just components.scss?

@addyosmani
Copy link
Contributor

^ I like components.scss. Any other votes?

@paulirish
Copy link
Member Author

Sgtm

@paulirish
Copy link
Member Author

  1. I still think you shouldn't use Article microdata when it's mobile-web-app-capable. If we were to suggest either, mobile-web-app-capable seems to be the bigger recommendation, no? What is article microdata giving us? Seems like it needs docs to justify if we think its worthwhile as a default.
  2. win8 tile images, still. :/
  3. precomposed icons neccessary to inline?
  4. i'm very happy the doctype is lowercase :)
  5. https://developers.google.com/webmasters/smartphone-sites/feature-phones is the docs for link rel canonical also LOL that canonical is desktop which is increasingly niche.
  6. <main> element, eh? MDN says its not supported by IE. Does that mean its inline-block by default there?

This was referenced Jun 18, 2014
Merged
Merged
addyosmani added a commit that referenced this issue Jun 18, 2014
@addyosmani
For #60 - add correct docs link for canonical
c0d8d32
@addyosmani
Copy link
Contributor

Thanks for your feedback once again, Paul. PR is in for component renaming. On your latest items:

  1. I have a PR in to remove the Article microdata and switch to mobile-web-app-capable as a default. After some research you're right. It doesn't make sense to prescribe the former here.
  2. I would like to keep these in for now. Whenever we're building sample apps that demonstrate multi-screen experiences we always end up testing on Win8 devices needing them. I'd rather devs (for now) just remember to generate them vs. ignoring completely, but we'll consider dropping to the wiki if there are lots of objections. Reasonable?
  3. Could you clarify what you mean? :) I understand inlining but are you suggesting static icons get inlined too?
  4. WEEEEEEEE.
  5. I have a PR in for adding that item to the comment. CRAZY that canonical is still desktop, but will try to find something recent which suggests otherwise.
  6. Folks generally seem to display:block <main> to work around issues in IE. We tackle this in the components.css/scss file at the moment.

sindresorhus added a commit that referenced this issue Jun 18, 2014
@sindresorhus
Merge pull request #70 from google/canonical
ba2acb2 
For #60 - add correct docs link for canonical
@addyosmani
Copy link
Contributor

@paulirish Closing up on this one as PRs for everything mentioned have been merged to master now. If there is anything else at all you feel still needs work please do holler!

@sindresorhus sindresorhus mentioned this issue Aug 6, 2014
Closed
@paulirish paulirish mentioned this issue Sep 23, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@passy @paulirish @addyosmani @sindresorhus