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

upkeep/170: bump WordPress tested up to from 6.0 to 6.1 #173

Merged
merged 8 commits into from
Feb 1, 2023

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented Nov 21, 2022

Description of the Change

Tested the following manually:

  • Characters from few categories
  • Enabled/Disabled Most Used palette
  • Clear Most Used palette

Closes #170

Changelog Entry

Security - bump WordPress tested up to from 6.0 to 6.1
Security - bump minimum supported WordPress version from 5.7 to 6.1

Credits

Props @Sidsector9

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@Sidsector9 Sidsector9 self-assigned this Nov 21, 2022
@Sidsector9 Sidsector9 requested review from a team and faisal-alvi and removed request for a team November 21, 2022 09:13
Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@Sidsector9 I have checked it in the WP v6.1 and the popup is going out of the viewport. However, when tested in WP v5.9, it is working fine. Please check the video.

insert-char-pr-173-not-working.mp4

@jeffpaul jeffpaul added this to the 1.0.6 milestone Dec 1, 2022
Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@Sidsector9 I have checked again in WP v6.1.1 and the popup is still overlapping but the right side and cut in half. Please check the video.

insert-char-pr-173-not-working.mp4

I'm ready to connect 1:1 to test and discuss this in detail.

@Sidsector9
Copy link
Member Author

@faisal-alvi that is a known issue with Popover. I have tried to make it work as much as possible, can you check?

@Sidsector9 Sidsector9 requested a review from iamdharmesh January 5, 2023 13:24
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @Sidsector9. I faced one strange error on WP 6.1, when we clear a Most Used palette and try to insert a special character, it didn't add a character and shows the following error in the console. this is happening with 6.1 only. working fine on WP 6.0.

Uncaught TypeError: Cannot read properties of undefined (reading 'count')
    at a.value (index.js?ver=45a2f9e69c15e444b912:1:5708)
    at a.value (index.js?ver=45a2f9e69c15e444b912:1:5059)
    at onClick (index.js?ver=45a2f9e69c15e444b912:1:7721)
    at HTMLUnknownElement.callCallback (react-dom.js?ver=17.0.1:3942:16)
    at Object.invokeGuardedCallbackDev (react-dom.js?ver=17.0.1:3991:18)
    at invokeGuardedCallback (react-dom.js?ver=17.0.1:4053:33)
    at invokeGuardedCallbackAndCatchFirstError (react-dom.js?ver=17.0.1:4067:27)
    at executeDispatch (react-dom.js?ver=17.0.1:8273:5)
    at processDispatchQueueItemsInOrder (react-dom.js?ver=17.0.1:8305:9)
    at processDispatchQueue (react-dom.js?ver=17.0.1:8318:7)

Screenshot 2023-01-06 at 6 18 23 PM

Could you please help to check it once?

Thanks.

@Sidsector9 Sidsector9 requested review from jeffpaul and a team as code owners January 25, 2023 11:47
iamdharmesh
iamdharmesh previously approved these changes Jan 26, 2023
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the issue @Sidsector9, it looks good now.

Just wanted to add 2 notes here, otherwise, all is good and approved from my side.

  • WP minimum version updated to 6.1 (as we discussed this is updated to fix popover position issue)
  • I am seeing a warning when we open popover, but it seems minor deprecation warning and can be fixed in upcoming PRs.
    image

@jeffpaul
Copy link
Member

@Sidsector9 if you want to resolve the merge conflict here, we're good to otherwise merge this PR

@Sidsector9
Copy link
Member Author

@jeffpaul dependency review still fails because the license in mousetrap package is combined as Apache-2.0 AND Apache-2.0 WITH LLVM-exception. We allow-listed both separately here https://github.com/10up/.github/pull/26/files.

Wondering if we should add Apache-2.0 AND Apache-2.0 WITH LLVM-exception to the allow-list?

@jeffpaul
Copy link
Member

jeffpaul commented Feb 1, 2023

Yeah, I'll go ahead and merge this in now and amend the license policy next

@jeffpaul jeffpaul merged commit 105dc37 into develop Feb 1, 2023
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.

The plugin hasn't been tested with the latest version of WordPress
4 participants