Show notice in Course Editor to focus on Course Outline block or Insert one if block does not exist for first time users #7364
Conversation
WordPress Dependencies ReportThe
This comment was automatically generated by the |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## trunk #7364 +/- ##
============================================
- Coverage 50.95% 50.93% -0.03%
- Complexity 11156 11161 +5
============================================
Files 613 614 +1
Lines 47073 47163 +90
Branches 404 413 +9
============================================
+ Hits 23986 24021 +35
- Misses 22760 22808 +48
- Partials 327 334 +7
Continue to review full report in Codecov by Sentry.
|
|
Hey @Imran92! Did a super quick test.
I'm thinking perhaps we should change it to:
If I saw that notice that asked me to add some lessons, and I did that, I'd be confused as to why the notice was still there. Then, I think we could catch the issue with draft lessons by tweaking this frontend notice and adding a CTA (though as part of a separate issue and PR):
I created 2 courses and didn't add lessons to either of them, but I saw the notice in both. After I published a lesson in the first course, I still saw the notice in the second course. I'm happy to go with whichever of these is easiest to implement:
Please let me know your thoughts on all of this. 🙇🏻♀️ |
… and 'add/notice-to-create-course-outline-block-first-time' of https://github.com/Automattic/sensei into add/notice-to-create-course-outline-block-first-time
|
Thanks for checking it @donnapep !
I've updated it here ddb29ea, now the notice will only be shown after a pattern is selected
Agreed, I'm updating it here 4bc987c to show the notice only when there are no lessons in it.
Oh, it was only considering if you've published a course. I've updated the logic here f424669 to consider any Course of any status. Showing it the first time sounds better as it helps with having less notice cluttering on the top.
Absolutely agree with this one. After seeing the deep dive, I think any improvement here that helps with publishing the course and lessons properly is worth doing. |
|
@Imran92 It's better, but still isn't quite working as expected for me. The notice is now showing after a course layout is selected, but if I save the course without adding any lessons, and then come back in later, the notice isn't there anymore. Also, looks like some tests are broken. |
|
Thanks for taking a look @donnapep ! I've updated the logic here e84c62a to load the notice scripts only when the user has at least a lesson created under a course. Here, I've gone with the following approach, as it required less checking with db query
I've left some edge case behaviors, but wanted to surface them here to get your opinion on them -
Thanks! |
|
@Imran92 Thanks! The logic looks good. I had a bit of confusion when testing. At one point, the notice stopped showing for new users. 😱 After stepping through code, I realized it was because I had previously dismissed the notice, which I see is tracked in local storage. Is this how WordPress typically handles client-side notices? I know for server-side it's usually an option in the An FYI that I haven't done a code review; I've only tested the behavior & logic. Given our discussion about not really needing this for when a user selects a pattern from the block inserter, if there is any feedback from a code review that requires reworking the patterns code, please just remove that logic. It's not part of the flow that new users are most likely going to move through, and so is not the flow we're most interested in capturing. No sense in spending any more time there. 🙂 I'm also hoping this notice will be temporary until we're able to secure a design resource to help us with this. Thx! |
There was a problem hiding this comment.
In general, it looks good.
But I'd like to propose something. IMO, this is a very small helper which doesn't worth such complexity to maintain. My suggestion would be:
- Don't worry about persisting the dismiss, since it would simplify the code, and it's a helper to "jump" to the Course Outline for new courses.
- Since we won't display the notice when having draft lessons, create the notice as soon as the wizard is closed (skipping earlier or choosing a pattern), and remove the notice when:
- Clicking on "add some lessons".
- When clicking on the "X".
- When clicking on the course outline actions "Start with blank" or "Generate with AI".
With that, we don't need to:
- Monitor the dispatch.
- Subscribe the store.
- Have any backend logic.
- Have new JS assets (we can have a file imported in the
course-edit.js). - Worry about persistency.
- Worry if the post is new or not.
WDYT? cc @donnapep
| const { insertBlock } = dispatch( 'core/block-editor' ); | ||
|
|
||
| courseOutlineBlock = createBlock( 'sensei-lms/course-outline' ); | ||
| await insertBlock( courseOutlineBlock ); |
There was a problem hiding this comment.
I didn't test it, but I think the selection happens by default given the documentation (updateSelection defaults to true).
If confirmed, I think it doesn't need to be an async function, and we can add the following selection in an else. WDYT?
There was a problem hiding this comment.
Makes sense, I've updated the code here a633f08
There was a problem hiding this comment.
Thank you @Imran92!
With this, I think the function doesn't need to be async anymore, right?
| let noticeRemoved = false; | ||
|
|
||
| const notice = __( | ||
| 'Nice! Now you can <a href="javascript:;" onclick="window?.handleCourseOutlineBlockIncomplete();">add some lessons</a> to your course.', |
There was a problem hiding this comment.
My suggestion wouldn't match the design, but it would match the default behavior of the Gutenberg notices.
Should we tweak it to use a separate button, being the actions from the Gutenberg notice instead? cc @donnapep
So we wouldn't need to put the function in the window, we wouldn't need to add the onclick and href="javascript:;" as HTML attributes, it would get more organized in terms of code, and the users would the same behavior of other notices.
If we decide to not change it, we need to extract the HTML and logic from the translation string.
There was a problem hiding this comment.
If we're good with the design change, we can go for it 👍 Our current design of showing it as a URL looks better to me. But as showing it as button is simpler, as that's the default behavior. I think the main benefit of this will be not having to use the __unstableHTML property.
There was a problem hiding this comment.
I'm fine with changing it to use a button if it helps simplify things. In that case, we could change the message to:
Nice! Now you can add some content to your course. Add Lessons
where Add Lessons is a button that jumps to / adds the Course Outline block.
Let's see what that looks like. 🙂
|
Thanks for checking @donnapep!
Yap you are right, if it's a global notice to be shown once, it's saved in
Thanks! I've removed it here ee75e87 . |
|
Thanks @renatho for your nice suggestion and the new approach you suggested, I'm good with having a version with a bit of a smaller set of requirements for it. My only concern is I'm not sure users who are familiar with the Sensei environment (from the perspective of our current set of requirements, they are users who have created any lesson that belongs to any course) would like to see a notice on top. And also, even though we have an extra asset, it's loaded only for the specific case of the first Course. A little benefit that I like about this is we load less code if all subsequent usages. But I'm okay to load it as part of Thanks again! <3 |
Otherwise php above 8 throws an error in testing. So we manually set the _new_post to true for new post. If we do it in backed, it causes the new course modal to appear for existing courses
After thinking about this for a bit, if Renatho's suggestions #7364 (review) significantly simplify the logic, I'm open to displaying this notice for existing users too. I would want to tweak the messaging though:
where "Add some now" is a button that jumps to / adds the Course Outline block. |
|
We're pausing this work until we get through some of the other onboarding improvements. |
Resolves #7331
Proposed Changes
New users often don't know where they can add lessons, or sometimes, they don't know about the need of Course Outline block. So here, we've added a notice in the Course Editor that is shown to only those users who have not created any course yet.
Clicking on the link on that notice will focus (and scroll to, if necessary) on the Course Outline block if there is already one added in the editor.
If the Course Outline block is not there in the editor, it'll create one for the user. If at least one of the lessons in the Course Outline block is published, this notice will disappear.
To not bother the existing users, we've taken some measures. If the user has authored any course, this notice isn't shown to them. Once the user dismisses the notice, it's not shown to them anymore.
One thing to be noted is, that the dismissed flag is stored in localStorage. So the user may see it in a different browser or device. But it may not be very likely because once the user is in the Course editor, and playing around with it, there's a high probability that they'll have authored at least a course. So I didn't want to add another user meta for it as it may turn out to be not very useful.
I'll add some more tests for testing all the edge cases either in this PR, or a new one. I'm temporarily moving on to another issue.
Testing Instructions
add some lessonslink on the noticeScreen.Recording.2023-12-06.at.11.51.46.PM.mov
Pre-Merge Checklist