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

Should labelTagName default to LABEL? #759

Closed
zepumph opened this issue Mar 31, 2018 · 4 comments
Closed

Should labelTagName default to LABEL? #759

zepumph opened this issue Mar 31, 2018 · 4 comments
Assignees
@mbarlow12
Labels
status:ready-for-review

Comments

@zepumph
Copy link
Member

zepumph commented Mar 31, 2018

In #748 I made it default to LABEL, but perhaps it should be a p or something. @jessegreenberg @mbarlow12 what do you think? What is the best default?
@jessegreenberg
Copy link
Contributor

label is really only only good for input tags, I would prefer p.

@jessegreenberg jessegreenberg removed their assignment Apr 3, 2018
@zepumph
Copy link
Member Author

zepumph commented Apr 3, 2018

I like your recommendation greatly. It aligns with the same thought process between redoing prependLabels (really input-centric) to appendLabel/appendDescription (more streamlined with general use case which isn't the input/label cobo).

@mbarlow12 as long as there are no objections I will make the default a p. Thanks @jessegreenberg.

@zepumph zepumph self-assigned this Apr 4, 2018
zepumph added a commit to phetsims/rutherford-scattering that referenced this issue Apr 8, 2018
@zepumph
add labels back in because it isn't the default, phetsims/scenery#759, …
4cb1f42 
…phetsims/scenery#753
zepumph added a commit to phetsims/resistance-in-a-wire that referenced this issue Apr 8, 2018
@zepumph
add labels back in because it isn't the default, phetsims/scenery#759, …
89e046c 
…phetsims/scenery#753
zepumph added a commit to phetsims/john-travoltage that referenced this issue Apr 8, 2018
@zepumph
add labels back in because it isn't the default, phetsims/scenery#759, …
bf7a8ee 
…phetsims/scenery#753
zepumph added a commit to phetsims/scenery-phet that referenced this issue Apr 8, 2018
@zepumph
add labels back in because it isn't the default, phetsims/scenery#759, …
c8a3348 
…phetsims/scenery#753
zepumph added a commit to phetsims/ohms-law that referenced this issue Apr 8, 2018
@zepumph
add labels back in because it isn't the default, phetsims/scenery#759, …
953f051 
…phetsims/scenery#753
zepumph added a commit that referenced this issue Apr 8, 2018
@zepumph
default label tag name to 'p',#759
67df21a
@zepumph
Copy link
Member Author

zepumph commented Apr 8, 2018

I changed the default label tag name to 'p', but first I had to go through commits made in #748 called "remove unneeded calls to options with default values". In those commits I removed "labelTagName: 'label'` options. Manually reverted above.

@mbarlow12 could you please review this.

@zepumph zepumph removed their assignment Apr 8, 2018
@mbarlow12
Copy link
Contributor

Yikes this is old! @zepumph all looks good. Closing.

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

@mbarlow12 mbarlow12

Labels
status:ready-for-review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mbarlow12 @jessegreenberg @zepumph