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

Components are now accessible via camelCase and UpperCamelCase me… #3439

Merged
merged 3 commits into from
Nov 3, 2016

Conversation

chemoish
Copy link
Member

Fixes #3436.

Description

Components are not uniformly accessible through the same names.

Example:

  • getChild cannot retrieve a component by a camelCase name
  • addChild can retrieve, then add a component by a camelCase name

Specific Changes proposed

Enable components to be accessed via both camelCase and UpperCamelCase means.

Requirements Checklist

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

…ans.

  • addChild
  • getChild
  • getComponent
  • registerComponent

@@ -361,7 +361,7 @@ class Component {

// If no componentClass in options, assume componentClass is the name lowercased
Copy link
Member Author

Choose a reason for hiding this comment

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

comment might need to be updated.

Copy link
Member

Choose a reason for hiding this comment

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

I think this comment could just be removed?

@chemoish chemoish force-pushed the feature/fix-component-sensitivity branch from de10475 to ff54edc Compare July 23, 2016 00:43
@gkatsev
Copy link
Member

gkatsev commented Aug 8, 2016

Looks like this needs to be rebased against master

@chemoish chemoish force-pushed the feature/fix-component-sensitivity branch from ff54edc to 78db9f2 Compare August 8, 2016 22:23
@chemoish
Copy link
Member Author

chemoish commented Aug 8, 2016

@gkatsev I temporarily removed vjsstandard and rebased master.

…ans.

- addChild
- getChild
- getComponent
- registerComponent
@chemoish chemoish force-pushed the feature/fix-component-sensitivity branch from 78db9f2 to f94c3d9 Compare September 26, 2016 18:24
@chemoish
Copy link
Member Author

@gkatsev updated again.

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Basically, components will be stored internally via the toTitleCase name?
LGTM.
Test failing because of the firefox issue.

@gkatsev gkatsev added this to the 5.13 milestone Sep 27, 2016
@chemoish
Copy link
Member Author

@gkatsev, Is that a known issue? I wasn't having that issue with FF 48, but I am having it with 49.0.1, does that have to do with me?

(removing the extraneous comment)

@gkatsev
Copy link
Member

gkatsev commented Sep 28, 2016

@chemoish yup, I have a PR to fix the issue that should be merged soon: #3640

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

LGTM and works for me.

@gkatsev gkatsev added confirmed minor This PR can be added to a minor release. It should not be added to a patch release. labels Oct 3, 2016
@gkatsev gkatsev merged commit 9d77268 into videojs:master Nov 3, 2016
@hartman
Copy link
Contributor

hartman commented Nov 3, 2016

@gkatsev does it per chance also effect #2901 ?

@gkatsev
Copy link
Member

gkatsev commented Nov 3, 2016

@hartman unfortunately, it doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed minor This PR can be added to a minor release. It should not be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

addChild should not be sensitive to the first letter casing
4 participants