Brought back icon density member (closes #118) #222

Merged
merged 4 commits into from Jun 16, 2014

Conversation

Projects
None yet
2 participants
@marcoscaceres
Member

marcoscaceres commented Jun 10, 2014

No description provided.

@marcoscaceres

This comment has been minimized.

Show comment Hide comment
@marcoscaceres

marcoscaceres Jun 10, 2014

Member

@mrdewitt can you review?

Member

marcoscaceres commented Jun 10, 2014

@mrdewitt can you review?

index.html
+ device pixel density for which this icon was designed. The device
+ pixel density is expressed as the number of dots per 'px' unit
+ (equivalent to a dppx as defined in [[!css3-values]]). The value is a
+ positive number greater than 0. If the developer ommits the value,

This comment has been minimized.

Show comment Hide comment
@mrdewitt

mrdewitt Jun 11, 2014

s/ommits/omits/

@mrdewitt

mrdewitt Jun 11, 2014

s/ommits/omits/

index.html
@@ -576,10 +578,15 @@
the <a>steps for processing a <code>sizes</code> member of an
icon</a> passing <var>potential icon</var>.
</li>
+ <li>Let <var>density</var> be the list that result from

This comment has been minimized.

Show comment Hide comment
@mrdewitt

mrdewitt Jun 11, 2014

I thought that |density| was a single number for a given icon candidate, not a list.

@mrdewitt

mrdewitt Jun 11, 2014

I thought that |density| was a single number for a given icon candidate, not a list.

This comment has been minimized.

Show comment Hide comment
@marcoscaceres

marcoscaceres Jun 11, 2014

Member

D'oh! Copy/pasta error.

@marcoscaceres

marcoscaceres Jun 11, 2014

Member

D'oh! Copy/pasta error.

index.html
- whose values are <var>src</var>, <var>type</var> and
- <var>sizes</var> respectively.
+ <code>src</code>, <code>type</code>, <code>sizes</code>, and
+ <code>density</code> whose values are <var>src</var>,

This comment has been minimized.

Show comment Hide comment
@mrdewitt

mrdewitt Jun 11, 2014

This statement seems confusing and self-referential (the object keys have value of themselves?). I understand that when formatted, the and the tags appear different but still reads strangely.

@mrdewitt

mrdewitt Jun 11, 2014

This statement seems confusing and self-referential (the object keys have value of themselves?). I understand that when formatted, the and the tags appear different but still reads strangely.

This comment has been minimized.

Show comment Hide comment
@marcoscaceres

marcoscaceres Jun 11, 2014

Member

agree. Will rewrite this.

@marcoscaceres

marcoscaceres Jun 11, 2014

Member

agree. Will rewrite this.

@mrdewitt

This comment has been minimized.

Show comment Hide comment
@mrdewitt

mrdewitt Jun 11, 2014

Thanks for writing this up! The example manifest should also be updated to include the density property.

Thanks for writing this up! The example manifest should also be updated to include the density property.

Addressed @mrdewitt's comments from #222
Also fixed a few small bugs:
 * the algo was stopping if the type was undefined
 * there is no need to check for -Infinity or -0, just for less than
+0.
@marcoscaceres

This comment has been minimized.

Show comment Hide comment
@marcoscaceres

marcoscaceres Jun 13, 2014

Member

@mrdewitt, I've updated the PR to address your comments. Hopefully all good now. Give me a 👍 if you are ok for me to merge.

Member

marcoscaceres commented Jun 13, 2014

@mrdewitt, I've updated the PR to address your comments. Hopefully all good now. Give me a 👍 if you are ok for me to merge.

marcoscaceres added a commit that referenced this pull request Jun 16, 2014

Merge pull request #222 from w3c/density
Brought back icon density member (closes #118)

@marcoscaceres marcoscaceres merged commit 63d015e into gh-pages Jun 16, 2014

@marcoscaceres

This comment has been minimized.

Show comment Hide comment
@marcoscaceres

marcoscaceres Jun 16, 2014

Member

Merging to move forward on this.

Member

marcoscaceres commented Jun 16, 2014

Merging to move forward on this.

@marcoscaceres marcoscaceres deleted the density branch Oct 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment