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

Add editor wizard modal for courses and lessons #5149

Merged

Conversation

renatho
Copy link
Contributor

@renatho renatho commented May 17, 2022

Fixes #5125

Changes proposed in this Pull Request

  • It removes _needs_template logic (old logic to add template when created through Course Outline).
  • It removes Learning Mode onboarding.
  • It adds a modal when creating new Courses and Lessons (it also applies to lessons created dynamically through the Course Outline).
  • This PR doesn't add styles to the modal. I'm leaving it to the "steps" issues.
  • It refactors the message we show to the user when adding/removing Sensei blocks to the post editor. Discussion: p1652822680825379-slack-C07418EJ0
  • The Sensei Pro product tour will also be removed as part of another PR in Sensei Pro.

Testing instructions

  • Clear the Local Storage.
  • If you're using a more recent version of Gutenberg, remove user meta wp_persisted_preferences (more details).
  • Create a new Course, and a new Lesson through the "New Course" and "New Lesson" buttons in their respective sections under WP Admin. Make sure you see a modal when creating them. Also, make sure that they don't conflict with the Gutenberg blocks modal from the core (our modal is just displayed after closing the Gutenberg modal).
  • Close the modal, save the post, and refresh the page. Make sure the modal doesn't appear again.
  • Now, create a lesson through the Course Outline.
  • Edit the lesson, and make sure the modal is displayed.
  • Close the modal, and make sure it adds the default blocks template to the Lesson.
  • Save the post, refresh the page, and make sure the modal isn't displayed again.

Screenshot / Video

Screen.Recording.2022-05-17.at.18.41.12.mp4

@renatho renatho added this to the Course and Lesson Wizard milestone May 17, 2022
@renatho renatho self-assigned this May 17, 2022
@renatho renatho changed the base branch from trunk to feature/course-lesson-wizard May 17, 2022 18:44
@renatho renatho force-pushed the add/course-lesson-editor-wizard-modal branch from 5d18c0c to 5713ce0 Compare May 17, 2022 20:07
@renatho renatho requested a review from a team May 17, 2022 21:48
@renatho renatho marked this pull request as ready for review May 17, 2022 21:49
Copy link
Contributor

@aaronfc aaronfc left a comment

Choose a reason for hiding this comment

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

Looks good, works well!

The only thing I noticed is that closing the modal will not make any effect unless you save or publish the post (lesson or course). But it's working like that in all scenarios (new lesson, new course and editing a draft lesson for first time) so probably it is expected behaviour?

@aaronfc aaronfc requested a review from a team May 18, 2022 09:54
@renatho renatho merged commit 5525b06 into feature/course-lesson-wizard May 18, 2022
@renatho renatho deleted the add/course-lesson-editor-wizard-modal branch May 18, 2022 15:35
@renatho
Copy link
Contributor Author

renatho commented May 18, 2022

The only thing I noticed is that closing the modal will not make any effect unless you save or publish the post (lesson or course). But it's working like that in all scenarios (new lesson, new course and editing a draft lesson for first time) so probably it is expected behaviour?

I think so! Maybe @burtrw could confirm it?

Ronnie, in summary (@aaronfc, please correct me if I'm missing something), when we close the modal, or select a pattern in the future implementations, and refresh the page without saving it, it will display the modal again because it continues being a new post. Is it the expected behavior? A workaround we could implement, is force saving the post after closing the modal or selecting a pattern. But not sure it would be an expected behavior from the user. 🤔

See how it works for pages that is being implemented for WP 6.0.

Closing the modal before selecting a pattern, and then refresh the page (in the second 10 I'm refreshing the page)
https://user-images.githubusercontent.com/876340/169084901-27167bc5-cb0d-4487-b485-91c55f5795f5.mov

Closing the modal after selecting a pattern, and then refresh the page.
https://user-images.githubusercontent.com/876340/169085628-44b2cb64-6f9c-4509-b180-488be1119d7e.mov

I can see that it's a different behavior though, since we have some more steps.

Update: Sorry! Not sure what happened that GH is not embedding the video, and just adding the link. 🤷‍♂️

@burtrw
Copy link
Member

burtrw commented May 18, 2022

One reason we may want to force save the post is that the first step in the wizard is to add the Course name. If you do that, I think the user would expect to see that Course Name updated once you close the wizard. And then we wouldn't want to lose that or have to start back over. But maybe just by adding text to the title then it will be saved in the browser?

Otherwise, I'd go with whatever implementation will be fastest and lean towards the same behavior as pages in WP 6.0.

@aaronfc
Copy link
Contributor

aaronfc commented May 19, 2022

I think the simplest approach for me (as a user and also as a developer) would be to just do "nothing" in case I close the modal before I "complete" the wizard (complete here means choosing a pattern or the "blank template" in the last step). Any other action (closing the modal) would keep the default template as it is working now.

Related to the above, @burtrw, yesterday in the team call we talked briefly about the difference between the "default" pattern and the "blank pattern".
We assumed that "default" is the current set of blocks being inserted when creating a new lesson/course.
And "blank" I guess that would only set the title and ... maybe the description block?

@burtrw
Copy link
Member

burtrw commented May 20, 2022

I think the simplest approach for me (as a user and also as a developer) would be to just do "nothing" in case I close the modal before I "complete" the wizard (complete here means choosing a pattern or the "blank template" in the last step). Any other action (closing the modal) would keep the default template as it is working now.

I'm good with this, though I'd prefer if we could save the Course name if it is easy. But the priority for that would be low.

yesterday in the team call we talked briefly about the difference between the "default" pattern and the "blank pattern".
We assumed that "default" is the current set of blocks being inserted when creating a new lesson/course.
And "blank" I guess that would only set the title and ... maybe the description block?

Actually, I think they should be the same thing - let's forget 'blank' and only go with the default.

@renatho
Copy link
Contributor Author

renatho commented May 23, 2022

I'm good with this, though I'd prefer if we could save the Course name if it is easy. But the priority for that would be low.

@burtrw I think this would be easy to implement. Just confirming before creating an issue for that: We should save the post in both conditions (closing the modal or choosing a pattern), right?

Actually, I think they should be the same thing - let's forget 'blank' and only go with the default.

👍

@burtrw
Copy link
Member

burtrw commented May 23, 2022

Yep, correct, we should save the post in both conditions (closing the modal or choosing a pattern) :)

@renatho
Copy link
Contributor Author

renatho commented May 23, 2022

Yep, correct, we should save the post in both conditions (closing the modal or choosing a pattern) :)

Okay! Added as an implementation detail in the #5140 issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create modal for Course and Lesson editor Wizard
3 participants