jooking / closure-library

Automatically exported from code.google.com/p/closure-library
Apache License 2.0
0 stars 1 forks source link

idgenerator set different prefix #310

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago

What steps will reproduce the problem?
1.  Use goog.ui.IdGenerator

What is the expected output? What do you see instead?
I have use cases where I want a different prefix.
Eg: Flash FSCommand function names cannot have ":" in the name, but FSCommand 
functions depend on the id of the element.

What version of the product are you using? On what operating system?
Latest SVN

Please provide any additional information below.
I don't know if this addition is a necessary or justifiable change, but it 
happened to affect me.  I have attached the modifications I would make.  For 
myself I implemented my own class with a setPrefix function and a default 
prefix of "$".

Original issue reported on code.google.com by jhiswin on 12 Apr 2011 at 8:24

Attachments:

GoogleCodeExporter commented 9 years ago
I don't really get your use case.
Do you want to generate unique function names and store them in element ids?

Original comment by pall...@google.com on 27 Apr 2011 at 1:55

GoogleCodeExporter commented 9 years ago
Actually it's the other way around. FSCommand (Flash callback) requires that 
you create a function that has the element's id in the function name.  But yes. 
 Although FSCommand is supposedly superceded by ExternalInterface, I have code 
that requires it.  Also, FSCommand is more stable (does not cause weird 
errors/rendering problems).

But there are other issues, such as this:
http://code.google.com/p/swfobject/issues/detail?id=343
Their attitude appears to be to avoid colons in id names.

Original comment by jhiswin on 27 Apr 2011 at 4:24

GoogleCodeExporter commented 9 years ago
According to http://www.w3.org/TR/html40/types.html ids must start with a 
letter.

"ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by 
any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons 
(":"), and periods (".")."

The configurable prefix is an overkill, but it is fine to replace ":" with 
"id-" or similar.

Original comment by pall...@gmail.com on 27 Apr 2011 at 7:51

GoogleCodeExporter commented 9 years ago
I think HTML5 lifted all those restrictions.  But colons are used in CSS 
pseudo-selectors, so it would seem best to avoid colons.
Would it be possible to introduce the configurable prefix as a global?  
Something like goog.global.IDGENERATOR_PREFIX = "id-"?

Original comment by jhiswin on 27 Apr 2011 at 9:09

GoogleCodeExporter commented 9 years ago
Would this be better?  I have introduced a defined constant that can be 
overridden that should take effect at runtime.

Original comment by jhiswin on 3 May 2011 at 9:27

Attachments:

GoogleCodeExporter commented 9 years ago
Are custom prefixes really necessary?
The 'id-' prefix would support every use case, wouldn't it?

Original comment by pall...@gmail.com on 3 May 2011 at 10:40

GoogleCodeExporter commented 9 years ago
I don't know if it's necessary, but it doesn't hurt.  Just using id- would 
probably support all use cases.
I figure there must be some reason that the ":" prefix was used, maybe to be 
shorter or to avoid clashing with any id names, and having the prefix 
customizable doesn't do any harm or add any complexity.  And in the end, after 
Closure Compiler is done with it, it all ends up being the same.

Original comment by jhiswin on 4 May 2011 at 3:30

GoogleCodeExporter commented 9 years ago
I wouldn't like to bloat the library with features that nobody uses and nobody 
understands why it is there.

Original comment by pall...@google.com on 6 May 2011 at 9:49

GoogleCodeExporter commented 9 years ago
Well, it adds one constant in the readable code and it should be very easy to 
understand what it does.  It just seems like the best solution should there be 
any possible conflicts in id names.
For example if someone already has elements named el_* or whatever it is.

It's more like a constant defined in the source code that anyone will only ever 
notice if they need it.

Original comment by jhiswin on 6 May 2011 at 10:10

GoogleCodeExporter commented 9 years ago
But I do submit that if you'd rather have the code as short as possible, you 
just leave it as is.  Although, I don't see IdGenerator ever seeing any 
significant changes.

Original comment by jhiswin on 6 May 2011 at 10:12

GoogleCodeExporter commented 9 years ago

Original comment by nn...@google.com on 9 May 2012 at 10:01

GoogleCodeExporter commented 9 years ago

Original comment by nn...@google.com on 15 May 2012 at 10:47

GoogleCodeExporter commented 9 years ago
I have submitted an individual CLA, so this patch should be safe to incorporate 
now.

Original comment by jhiswin on 28 Oct 2013 at 9:51