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

Added property name to metadata - refs #66 #67

Merged
merged 5 commits into from
Jan 29, 2016

Conversation

joecritch
Copy link
Contributor

This adds the property name in a <code /> element, next to the detect title. This gives the benefit of not having to look up or guess the property when writing CSS or JS.

Refs #66

@patrickkettner
Copy link
Member

could you screenshot it real quick?

@joecritch
Copy link
Contributor Author

Sure, here you go.

pull-67-modernizr-neue

@patrickkettner
Copy link
Member

do you think its clear without a label explaining what that means?

On Thu, Dec 17, 2015 at 3:00 AM, Joe Critchley [email protected]
wrote:

Sure, here you go.

[image: pull-67-modernizr-neue]
https://cloud.githubusercontent.com/assets/24449/11868086/4a1ae978-a4ad-11e5-9df7-646864751c8c.png


Reply to this email directly or view it on GitHub
#67 (comment)
.

patrick

@ryanseddon
Copy link
Member

Maybe we could have Modernizr.<name> and .<name>

@stucox
Copy link
Member

stucox commented Jan 8, 2016

Or even something like

Usage:

if (Modernizr.history) {
  // supported
}
else {
  // not supported
}

Which would allow us to call out async detects as:

Modernizr.on('webp', function (result) {
  if (result) {
    // supported
  }
  else {
    // not supported
  }
});

@stucox stucox closed this Jan 8, 2016
@stucox stucox reopened this Jan 8, 2016
@joecritch
Copy link
Contributor Author

I think we should separate the concern of what the alias is vs. how to use it. Perhaps a progressive "?" help icon next to it that, when clicked, opens up usage examples with that alias in context?

Edit: Maybe not a "?" icon, but a "Usage" link?

@patrickkettner
Copy link
Member

@stucox I think the usage is a great idea, but not for this specific issue. It means the user looses a lot of the scan ability
@joecritch 👍

still not sure about how to show this the right way :/

@stucox
Copy link
Member

stucox commented Jan 8, 2016

Lateral thought:

Maybe the property name (history) should replace the title ("History API"), then the description could immediately follow (no need for a "Description" title) — and we make sure the first sentence of each description makes it very clear exactly what the detect does; preferably followed by a paragraph break.

@patrickkettner
Copy link
Member

would we change what shows on the main list?

@joecritch
Copy link
Contributor Author

— Made the property name into its own "Usage" box in the right-hand bar
— Added some basical modal functionality when you click "View examples"
— When async is true, it'll show a different JS example

screen shot 2016-01-27 at 15 08 38

Edit syntax highlighting would be nice? might require a heavy library though

@patrickkettner
Copy link
Member

looks great!

re syntax hihglighting, it shoulnd't need any kind of library at all. its all static content that is the same across all examples (as far as structure goes). We just need to pick a color scheme and add the proper classes to each element on the examples, right?

@joecritch
Copy link
Contributor Author

Good idea!

  • Added syntax highlighting
  • Improved code indent on async examples
  • Moved all CSS to stylesheets

screen shot 2016-01-28 at 13 47 44

@patrickkettner
Copy link
Member

Looks awesome! Do it to it

On Jan 28, 2016, at 5:48 AM, Joe Critchley [email protected] wrote:

Good idea!

Added syntax highlighting
Improved code indent on async examples
Moved all CSS to stylesheets


Reply to this email directly or view it on GitHub.

joecritch added a commit that referenced this pull request Jan 29, 2016
@joecritch joecritch merged commit 4c2e9c6 into Modernizr:master Jan 29, 2016
@joecritch
Copy link
Contributor Author

@patrickkettner merged to master, but it doesn't seem to have been deployed. might be related to #72

@patrickkettner
Copy link
Member

There is an issue with detects with multiple properties

image

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.

4 participants