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

Rename plugin name used in strings #2516

Merged
merged 10 commits into from Apr 22, 2019

Conversation

Projects
None yet
3 participants
@jom
Copy link
Member

commented Apr 17, 2019

This updates the plugin name to Sensei LMS in strings throughout the plugin.

Testing

  • Verify all strings are updated and nothing is broken.
  • Specifically, verify teachers have the same levels of access (only can access their own) on the following admin pages:
    • Grading
    • Analysis
    • Learner Management
    • Lessons (list)
    • Courses (list)
    • Questions (list)
    • Module order

Deprecations

The following filters have been deprecated in includes/class-sensei-list-table.php and replaced by new filters:

  • manage_sensei_page_sensei_learners_sortable_columns => manage_sensei-lms_page_sensei_learners_sortable_columns
  • manage_sensei_page_sensei_grading_sortable_columns => manage_sensei-lms_page_sensei_grading_sortable_columns
  • manage_sensei_page_sensei_analysis_sortable_columns => manage_sensei-lms_page_sensei_analysis_sortable_columns

@jom jom added this to the 2.0.0 milestone Apr 17, 2019

@jom jom requested review from alexsanford and donnapep Apr 17, 2019

@alexsanford
Copy link
Contributor

left a comment

Looks good! I picked up on a couple things:

Lengthening the name causes some issues in the email list signup modal (https://cld.wthms.co/HlTJqH). The title and the checkbox text now span multiple lines, and the title needs some spacing between those lines at least. Maybe widening the modal would work? Not sure what the implication is on mobile.

On the Data Updates page, all of the updates still say "Originally included in Sensei v1.x.x", instead of "Sensei LMS". Maybe this is fine since it was called "Sensei" for those versions, but we've also changed it to "Sensei LMS" in the descriptions of the updates, so they should probably be consistent?

@donnapep

This comment has been minimized.

Copy link
Collaborator

commented Apr 17, 2019

Maybe this is fine since it was called "Sensei" for those versions, but we've also changed it to "Sensei LMS" in the descriptions of the updates, so they should probably be consistent?

I'm fine with leaving things as they are given that Sensei wasn't named Sensei LMS back when the update was added. For me it also makes sense to have the description updated as was done in this PR to mention Sensei LMS as opposed to Sensei since that is the plugin that will run any future updates.

@donnapep

This comment has been minimized.

Copy link
Collaborator

commented Apr 17, 2019

Noting here that although I can access Order Modules as a teacher, I can't actually access the Modules page to create one. 😕This is in the current Sensei so nothing for this PR.

Have we decided to do anything about the broken hooks (re: https://a8c.slack.com/archives/C8QJAM52B/p1554818694005100)?

@jom

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

@alexsanford As a note to our Slack chat, I think Sensei LMS's is correct (apostrophe s after acronym ending with S, I think). I did a wee fix to fit it on 1 line on desktop and fixed line height for screens where it does smoosh. e23c478

@jom

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

@donnapep I don't see any usage in the wild for those three hooks (manage_sensei_page_sensei_learners_sortable_columns, manage_sensei_page_sensei_grading_sortable_columns, manage_sensei_page_sensei_analysis_sortable_columns), but I've added a deprecation in 71b6ce0. Any other hooks that use those screen IDs (inside WP core or in plugins) will break, but with how WP determines screen IDs, their usage should be discouraged.

Background trac on that issue:
https://core.trac.wordpress.org/ticket/21454
https://core.trac.wordpress.org/ticket/18857

@alexsanford
Copy link
Contributor

left a comment

Modal looks good! 👍

There's a linter issue we should address (commented below).

Show resolved Hide resolved includes/class-sensei-list-table.php
@donnapep
Copy link
Collaborator

left a comment

👍

@jom

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

@alexsanford Merging before your approval, but definitely encourage post-merge feedback :)

@jom jom merged commit d68cce7 into master Apr 22, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jom jom deleted the rename/plugin-sensei-lms-strings branch Apr 22, 2019

@donnapep donnapep modified the milestones: 2.0.0, 2.0.1 Apr 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.