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

Course.js: remove jquery and convert to typescript #3921

Merged
merged 16 commits into from
Aug 31, 2022

Conversation

BramDevlaminck
Copy link
Contributor

@BramDevlaminck BramDevlaminck commented Aug 23, 2022

This pull request removes jquery code out of course.js

progress on #3590, closes #3923 .

@BramDevlaminck BramDevlaminck added the chore Repository/build/dependency maintenance label Aug 23, 2022
@BramDevlaminck BramDevlaminck self-assigned this Aug 23, 2022
@BramDevlaminck BramDevlaminck changed the title chore: remove jquery out of parts of the code Remove jquery code Aug 23, 2022
@BramDevlaminck BramDevlaminck changed the title Remove jquery code Remove jquery code course.js Aug 24, 2022
app/assets/javascripts/course.js Fixed Show resolved Hide resolved
app/assets/javascripts/course.js Fixed Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2022

This pull request introduces 2 alerts when merging f52a4b1 into 7cce90f - view on LGTM.com

new alerts:

  • 2 for DOM text reinterpreted as HTML

@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2022

This pull request introduces 1 alert when merging 957be15 into 7cce90f - view on LGTM.com

new alerts:

  • 1 for DOM text reinterpreted as HTML

@BramDevlaminck BramDevlaminck marked this pull request as ready for review August 24, 2022 09:55
@BramDevlaminck BramDevlaminck requested a review from a team as a code owner August 24, 2022 09:55
@BramDevlaminck BramDevlaminck requested review from bmesuere and niknetniko and removed request for a team August 24, 2022 09:55
@BramDevlaminck BramDevlaminck marked this pull request as draft August 24, 2022 11:43
@BramDevlaminck BramDevlaminck changed the title Remove jquery code course.js Course.js: remove jquery and convert to typescript Aug 24, 2022
@BramDevlaminck BramDevlaminck marked this pull request as ready for review August 24, 2022 13:03
@BramDevlaminck BramDevlaminck mentioned this pull request Aug 24, 2022
9 tasks
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course.ts Outdated Show resolved Hide resolved
app/assets/javascripts/course_membership.js Outdated Show resolved Hide resolved
@chvp chvp added the deploy mestra Request a deployment on mestra label Aug 27, 2022
@chvp chvp temporarily deployed to mestra August 27, 2022 08:10 Inactive
@github-actions github-actions bot removed the deploy mestra Request a deployment on mestra label Aug 27, 2022
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

A minor nitpick: I think template strings (between backticks) would look cleaner for the selectors where you now have to escape the double quotes, e.g., in type=\"radio\".

@BramDevlaminck
Copy link
Contributor Author

A minor nitpick: I think template strings (between backticks) would look cleaner for the selectors where you now have to escape the double quotes, e.g., in type=\"radio\".

I've applied this and also changed the .eslintrc to allow this. With the previous .eslintrc eslint would not allow this since it forced you to use double quotes on every string

@bmesuere bmesuere merged commit e1a4fbc into develop Aug 31, 2022
@bmesuere bmesuere deleted the chore/jquery-removal branch August 31, 2022 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Repository/build/dependency maintenance
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor course.js to TypeScript and remove jQuery
4 participants