-
Notifications
You must be signed in to change notification settings - Fork 27.5k
feat (ng-pluralize): add alternative mapping using attributes #2454
Conversation
I'm not keen on this syntax as it requires the |
+1 for I am also rather drawn to the idea of using inner elements, which would allow us to have HTML and directives inside the templates. But working with transclusion in this way is rather problematic right now so probably for the future.
|
I like the idea of a |
@lgalfaso - Great! How are you getting on making the change to |
@petebacondarwin The change is simple, I am just trying to figure out how to handle negative numbers in a visually appealing way. Does |
Pete
|
@petebacondarwin Looks like a good alternative. Just push a new patch |
@@ -25,27 +25,40 @@ | |||
* and explicit number rules throughout later parts of this documentation. | |||
* | |||
* # Configuring ngPluralize | |||
* You configure ngPluralize by providing 2 attributes: `count` and `when`. | |||
* You configure ngPluralize by providing the attribute `count` any of the two ways to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is a bit unclear. Perhaps it could be something like this
You configure ngPluralize by providing attributes,
count
,when
andoffset
to describe what should be counted and how to map that count to templates for display.
And then enumerate the attributes and how they work below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks changed
Also, what happens if someone tries to use both strategies? It seems that at the moment both configurations are used with the |
@petebacondarwin Added the following comment to clarify what happens when the two mapping mechanisms are used It is possible to mix the two forms to define a mapping. In this case, if there is a conflict the definition used using |
@petebacondarwin Thanks for looking into this, and let me know if you would like me to rebase this into a single commit |
'<ng-pluralize count="email" ' + | ||
"when-minus-1='You have negative email. Whohoo!' " + | ||
"when-0='You have no new email' " + | ||
"one='You have one new email' " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be when-one
and when-other
below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to accept the unicode plural categories as-is, without the when-
prefix. Can change this to be when-
prefixed if you think it would make more sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when-...
throughout it definitely the way forward. It helps make it clear what you are talking about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this being consistent, changed
the rest looks good to me |
Please let me know if you would like me to rebase this into a single commit |
@lgalfaso yes, squasing commits would be helpful - would make it easier to review and merge |
@pkozlowski-opensource sure, done |
LGTM. @pkozlowski-opensource please merge |
on it |
@lgalfaso Sorry for being pain in a neck but 2ff7a69 is based on the 3-months old master and doesn't merge cleanly. I was trying to merge it in pkozlowski-opensource@f9c75e4 but after the merge there are several test failrues (different ones on different browsers), see: Not sure if this is due to my merges or there is something else going on so it would be awesome if you could have a look. Thnx! |
Add an alternative way to define a mapping for ng:pluralize using attributes instead of the `when` attribute Closes angular#2454
@pkozlowski-opensource 5e8dfe1 is a clean merge to master |
@lgalfaso merges cleanly now but there are 2 tests failures on IE8: When pushing a fix for those tests could you make sure that there is only one commit in the |
Add an alternative way to define a mapping for ng:pluralize using attributes instead of the `when` attribute Closes angular#2454
@pkozlowski-opensource I do not have access to an IE8, but I think that 9e5876f will make IE8 happy |
Bingo, lgalfaso@9e5876f did the trick. Landed as a170fc1. Thank you! |
Add an alternative way to define a mapping for ng:pluralize using attributes instead of the `when` attribute Closes angular#2454
When using ngPluralize, I found myself having to escape the characters " and '.
This PR adds an alternative way to define the mapping using attributes to the element like this