Skip to content

Replace jQuery logic with native DOM #1880

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

Merged
merged 7 commits into from
Nov 15, 2019
Merged

Replace jQuery logic with native DOM #1880

merged 7 commits into from
Nov 15, 2019

Conversation

locks
Copy link
Contributor

@locks locks commented Oct 26, 2019

No description provided.

@rust-highfive
Copy link

r? @smarnach

(rust_highfive has picked a reviewer for you, use r? to override)

@locks locks force-pushed the jquery-integration branch 2 times, most recently from cedbe79 to 1334323 Compare October 26, 2019 13:52
@locks locks changed the title replace triple mustache with html-safe helper Replace jQuery logic with native DOM Oct 26, 2019
Comment on lines +19 to +20
let $target = event.target;
let $c = this.element;
Copy link
Contributor

Choose a reason for hiding this comment

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

Both of them are not jQuery objects anymore. It would be better to remove $ from the names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left them as is to minimize the changes in the code. Also, this code will likely be changed or moved in a subsequent pass.

@carols10cents
Copy link
Member

Looks great! Locally, in Firefox, I tested:

  • API token input focusing
  • Sorting dropdown opening and closing
  • Readme syntax highlighting
  • Readme anchors

and everything worked as expected. Thanks!!!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 15, 2019

📌 Commit c6c87e1 has been approved by carols10cents

@bors
Copy link
Contributor

bors commented Nov 15, 2019

⌛ Testing commit c6c87e1 with merge 3248b4d...

bors added a commit that referenced this pull request Nov 15, 2019
@bors
Copy link
Contributor

bors commented Nov 15, 2019

☀️ Test successful - checks-travis
Approved by: carols10cents
Pushing 3248b4d to master...

@bors bors merged commit c6c87e1 into master Nov 15, 2019
@locks locks deleted the jquery-integration branch November 15, 2019 23:43
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.

6 participants