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

all components are case-sensitive #3416

Closed
wants to merge 1 commit into from
Closed

Conversation

si458
Copy link

@si458 si458 commented Jul 5, 2016

Description

the components do not work correctly unless every first charactor is lowercase and the rest uppercase

Specific Changes proposed

change first character to lowercase and rest uppercase

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

all components are case-sensitive so first letter must be lowercase!

all components are case-sensitive so first letter must be lowercase!
@@ -35,30 +35,30 @@ The actual default component structure of the Video.js player looks something li

```
Player
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea for this section to go with the "class names" rather than the instance names.

@gkatsev
Copy link
Member

gkatsev commented Jul 7, 2016

I wonder if we should make a note that when you do addChild or getChild it should be camel-cased instead.
Also, whether those method should just be case-insensitive?
@videojs/core-committers should addChild and related methods be case-insensitive for component names?

@misteroneill
Copy link
Member

I think they should be case-insensitive in the sense that they accept lowerCamelCase and UpperCamelCase because the latter matches the class names, but not UPPERCASE or lowercase.

@si458
Copy link
Author

si458 commented Jul 8, 2016

i think it be case-insensitive too :)

@gkatsev
Copy link
Member

gkatsev commented Jul 18, 2016

I created a new issue describing this and what we want to happen: #3436
So, going to close this PR. Thanks for your time @si458, if you feel up to it, would love it if you make a PR to fix #3436.

@gkatsev gkatsev closed this Jul 18, 2016
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.

3 participants