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

Added ADR #3 re: jQuery #2106

Merged
merged 2 commits into from
Aug 18, 2022
Merged

Added ADR #3 re: jQuery #2106

merged 2 commits into from
Aug 18, 2022

Conversation

stoyan
Copy link
Contributor

@stoyan stoyan commented Jul 7, 2022

No description provided.

## 🎬 Consequences
- Remove XSS vulnerability
- Lighter page

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the full list of dependencies just dialogs and tabs right now?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's some stuff in www/site.js
I saw a $.ajax

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a list of what I see in there:

  • drag and resizing (not sure where this is used)
  • viewport offsets
  • getting OS scrollbar width
  • displaying local time
  • tooltips
  • scroll handling
  • editting test labels



## 🤔 Decision
Remove jQuery and jQuery UI dependencies. Dialogs and tabs can be implemented in CSS with a sprinkle JavaScript.
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel like we can ditch the "dialogs and tabs" bit and expand a bit.

Something like....

"We can replace existing jQuery dependencies, and build new components, by focusing on a strong foundation of markup and CSS, with JavaScript sprinkled in as necessary for enhancement."

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, maybe we should make it explicit for anyone later that we intend on using vanilla JS at the moment.

Again...

"We will be using vanilla JavaScript, rather than immediately switching to another framework, to keep the performance overhead to a minimum."

Copy link
Contributor

@jefflembeck jefflembeck left a comment

Choose a reason for hiding this comment

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

Heck yes. Looks like this could be broken down into tickets and is a pretty easily definable project when we're ready

@stoyan stoyan merged commit 59871a6 into master Aug 18, 2022
@stoyan stoyan deleted the adr3 branch August 18, 2022 16:56
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.

None yet

4 participants