-
Notifications
You must be signed in to change notification settings - Fork 160
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
Get rid of jQuery dependency #589
Get rid of jQuery dependency #589
Conversation
@@ -16,9 +16,6 @@ | |||
}, | |||
"author": "Developer Express Inc.", | |||
"license": "MIT", | |||
"dependencies": { | |||
"jquery": "^2.0.0 || ^3.0.0" |
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.
What about types for jquery?
src/core/nested-option.ts
Outdated
|
||
if (renderData.container) { | ||
let container = renderData.container.get(0); | ||
let container = renderData.container.get ? renderData.container.get(0) : renderData.container; |
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.
Maybe we should extract a helper method with an understandable name?
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.
Isn't it a temporary solution, that already should be changed to DOM elements?
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.
No, it is not. You can add the jQuery integration into your application and API of DevExtreme widgets will change
src/core/template.ts
Outdated
@@ -44,22 +42,31 @@ export class DxTemplateDirective { | |||
'$implicit': renderData.model, | |||
index: renderData.index | |||
}); | |||
let container = renderData.container.get ? renderData.container.get(0) : renderData.container; |
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 same here
@@ -71,12 +71,12 @@ describe('DxTextBox value accessor', () => { | |||
fixture.componentInstance.formControl.disable(); | |||
fixture.detectChanges(); | |||
|
|||
expect(instance.option('disabled')).toBe(true); | |||
expect(instance['option']('disabled')).toBe(true); |
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.
Strange change
README.md
Outdated
@@ -137,6 +139,13 @@ npm start | |||
|
|||
Navigate to [http://127.0.0.1:8875/examples/](http://127.0.0.1:8875/examples/) in the opened browser window. Explore the **examples** folder of this repository for the examples source code. | |||
|
|||
### <a name="jquery integration"></a>Include jQuery integration ### | |||
DevExtreme has removed the jQuery dependence in version 17.2. It means that our widgets work without jQuery elements.If you need to use jQuery, you can include the jQuery integration as described 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.
Missed space after dot.
@@ -16,9 +16,6 @@ | |||
}, | |||
"author": "Developer Express Inc.", | |||
"license": "MIT", | |||
"dependencies": { | |||
"jquery": "^2.0.0 || ^3.0.0" | |||
}, | |||
"peerDependencies": { | |||
"devextreme": "~17.2.1-0", |
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 we update a devextreme
devDependency?
src/core/utils.ts
Outdated
public static getElement(element: any) { | ||
return element.get ? element.get(0) : element; | ||
}; | ||
} |
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.
Defining a class with static members is not a JavaScript way. Export static methods directly.
The jQuery dependency has been removed #46