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

Fix a few minor code issues #149

Merged
merged 1 commit into from Jun 22, 2023
Merged

Fix a few minor code issues #149

merged 1 commit into from Jun 22, 2023

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Jun 21, 2023

Description of the Change

While reviewing #148 I noticed there was some code that was no longer being used. I tracked that down to #129, where we refactored a few things but left some code in place that wasn't actually being used.

While putting together this PR to remove that code, I also noticed we had missing escaping in one place and that a couple of our text strings we show could be translated slightly better, so all of those have been fixed here.

So the things this PR does:

  1. Removes the code that references the screen_id, as the underlying code was removed in Add functionality to reset page order #129 but some code was left in place
  2. Update the reset confirmation message to use the post type on the PHP side (instead of replacing that on the JS side) and add a translator comment for that
  3. Add the post type to our reset message so it isn't hardcoded to always say post
  4. Add escaping around the post type value

How to test the Change

  1. Ensure you have a post type that is sortable (like the default page post type)
  2. Add multiple items within this post type
  3. Ensure you can sort these items by drag-and-drop
  4. Go to the Help tab and click on the Simple Page Ordering tab
  5. Ensure the reset message shows with the correct post type name
  6. Ensure clicking on this link shows a confirmation dialog with a message that reflects the proper post type
  7. Confirm this dialog and ensure post type sorting has been reset

Changelog Entry

Changed - Slightly change how some of our text is translated, passing in the post type
Fixed - Remove code that was no longer needed
Fixed - Add missing escaping

Credits

Props @dkotter

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.

@dkotter dkotter added this to the 2.6.0 milestone Jun 21, 2023
@dkotter dkotter self-assigned this Jun 21, 2023
@dkotter dkotter requested a review from a team as a code owner June 21, 2023 17:52
@dkotter dkotter requested review from ravinderk and removed request for a team June 21, 2023 17:52
@dkotter dkotter merged commit 5ae9e0e into develop Jun 22, 2023
10 checks passed
@dkotter dkotter deleted the fix/code-cleanup branch June 22, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants