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

[INT] Refactored some of the plugins - 3 #10946

Merged
merged 5 commits into from
May 8, 2024

Conversation

eva-vashkevich
Copy link
Member

@eva-vashkevich eva-vashkevich commented May 4, 2024

Summary

Fixes #10774
Refactored plugins to be Vue3 migration ready

Occurred changes and/or fixed issues

Refactored:

  • shell/plugins/clean-tooltip-directive.js
  • shell/plugins/directives.js
  • shell/plugins/int-number.js
  • shell/plugins/clean-html-directive.js

Technical notes summary

Areas or cases that should be tested

The entire application is affected since these plugins are globally used.
In particular:

  • html tags should be sanitized (all hyperlinks should have 'noopener', 'noreferrer', 'nofollow' attributes)
  • tooltips should look and work as expected
  • focusing on elements should work as before
  • keypress on number should work as before

Areas which could experience regressions

The entire application is affected since these plugins are globally used.

Screenshot/Video

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@eva-vashkevich eva-vashkevich requested a review from rak-phillip May 4, 2024 00:24
@eva-vashkevich eva-vashkevich self-assigned this May 4, 2024
@eva-vashkevich eva-vashkevich added this to the v2.9.0 milestone May 4, 2024
@eva-vashkevich eva-vashkevich added the QA/manual-test Indicates issue requires manually testing label May 6, 2024
rak-phillip
rak-phillip previously approved these changes May 7, 2024
Copy link
Member

@rak-phillip rak-phillip left a comment

Choose a reason for hiding this comment

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

LGTM

shell/plugins/directives.js Outdated Show resolved Hide resolved
Copy link
Member

@rak-phillip rak-phillip left a comment

Choose a reason for hiding this comment

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

This is looking great. I just have one concern about how we export some of our directives.

Copy link
Member

Choose a reason for hiding this comment

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

Another thought about naming, we could probably categorize all of our directives under shell/directives.. this can be done as a future PR, I feel this PR has enough content in it already.

shell/plugins/focus.js Show resolved Hide resolved
@eva-vashkevich eva-vashkevich requested a review from rak-phillip May 8, 2024 21:15
@eva-vashkevich eva-vashkevich merged commit 6bd67ba into rancher:master May 8, 2024
28 checks passed
@eva-vashkevich eva-vashkevich deleted the iss10774-p03 branch May 8, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extensions QA/manual-test Indicates issue requires manually testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor Plugins and Directives to Provide a Path for Vue3
2 participants