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

Closes #7075: Plugin - Accommodate new oneclick exclusions UI for delayjs #7090

Conversation

jeawhanlee
Copy link
Contributor

Description

Fixes #7075
Explain how this code impacts users.

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

Detailed scenario

N/A

Technical description

Documentation

This code adds a parent category to the scripts and adapts compatibility with the new delayjs backend exclusion list structure. it also adds some enhancement to the UI/UX.

New dependencies

N/A

Risks

N/A

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I did not introduce unnecessary complexity.

Additional Checks

  • In the case of complex code, I wrote comments to explain it.
  • I added error handling logic when using functions that could throw errors (HTTP/API request, filesystem, etc.)

@jeawhanlee jeawhanlee added the epics 🔥 For large tasks or features, broken into smaller issues. label Nov 5, 2024
@jeawhanlee jeawhanlee added this to the 3.18 milestone Nov 5, 2024
@jeawhanlee jeawhanlee self-assigned this Nov 5, 2024
@jeawhanlee jeawhanlee linked an issue Nov 5, 2024 that may be closed by this pull request
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for c2e65f01 85.58% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (c2e65f0) Report Missing Report Missing Report Missing
Head commit (64d522b) 38186 16801 44.00%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#7090) 104 89 85.58%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@jeawhanlee
Copy link
Contributor Author

jeawhanlee commented Nov 5, 2024

Old UI

Screenshot 2024-11-05 at 14 31 06

New UI Changes

Screenshot 2024-11-05 at 14 28 14

@jeawhanlee
Copy link
Contributor Author

jeawhanlee commented Nov 5, 2024

Automated testing passes with the new data structure, awaiting backend implementation to be sure it truly passes for manual testing.

@jeawhanlee
Copy link
Contributor Author

Icons implementation pending.
CC @piotrbak

@jeawhanlee jeawhanlee marked this pull request as ready for review November 5, 2024 13:42
@jeawhanlee jeawhanlee requested a review from a team November 8, 2024 09:47
@piotrbak
Copy link
Contributor

piotrbak commented Nov 8, 2024

@jeawhanlee I don't have access to this Figma. Checking with @DahmaniAdame if he has

@DahmaniAdame
Copy link
Contributor

@jeawhanlee I don't have access to this Figma. Checking with @DahmaniAdame if he has

@piotrbak I don't. Ben was the only one with access to it.

@jeawhanlee
Copy link
Contributor Author

@wp-media/qa-team For testing you can replace this line with the last endpoint in the comment here and update lists from tools section in WPR dashboard.

@jeawhanlee jeawhanlee requested a review from a team December 10, 2024 09:56
Copy link
Contributor

@wordpressfan wordpressfan left a comment

Choose a reason for hiding this comment

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

Icons image sizes are not exactly the same which is causing not good UI:

image

Can we make sure that the size is 16X16 so it has no UI problems?
also I can see that not all of them are images but some of them are dashicons so make sure that their width and height are exactly the same too and if it's not working properly with dashicons convert all of them to be images.

what do u think?

@Mai-Saad
Copy link
Contributor

Mai-Saad commented Dec 13, 2024

@jeawhanlee Thanks for the update, rollback is good now. But Ads and other icons do not match the ones here #7090 (comment)
Screenshot from 2024-12-13 10-27-10

@jeawhanlee
Copy link
Contributor Author

@Mai-Saad This is because they don't match the redesign on notion

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.20% (target: -0.10%) 82.14% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ce94d1f) 38262 16746 43.77%
Head commit (00e7f1c) 38344 (+82) 16860 (+114) 43.97% (+0.20%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#7090) 112 92 82.14%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@Mai-Saad
Copy link
Contributor

Thanks @jeawhanlee , Icons are matching the slack thread now
Screenshot from 2024-12-13 15-03-05

@Mai-Saad Mai-Saad merged commit 9810364 into feature/3.18 Dec 13, 2024
12 checks passed
@Mai-Saad Mai-Saad deleted the feature/7075-3.18-accommodate-new-oneclick-exclusions-ui-for-delayjs branch December 13, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epics 🔥 For large tasks or features, broken into smaller issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin - Accommodate new oneclick exclusions UI for delayjs
6 participants