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 facilitator note #3052

Open
wants to merge 26 commits into
base: trunk
Choose a base branch
from
Open

Add facilitator note #3052

wants to merge 26 commits into from

Conversation

renintw
Copy link
Contributor

@renintw renintw commented Dec 4, 2024

Resolves #2869

This PR introduces the UI for Facilitator Notes, including updates for mobile view. Additionally, the Breadcrumbs section has been changed from "Home" to "Learn WordPress."

Screenshots

Screen.Capture.on.2024-12-10.at.17-32-12.mov
Desktop Mobile
Editor image -
Frontend image image
  1. Based on this comment, which mentions:

The desired end state for 'Teacher’s Notes' is a tab or popup sidebar section within a lesson that can host the details of a lesson plan for a facilitator. The information of the lesson plan would be stored in a custom field / block within the lesson editor.

A Lesson Facilitator Note block has been added, which can be used within the Lesson Editor to select the corresponding lesson plan. The content can be edited and saved directly within the editor.
After saving the post, a Facilitator Notes label will be displayed in the previously discussed location. Clicking on this label will open a new tab showing the lesson plan.
And this block can be added anywhere within the post without affecting the final render of the article.

  1. Based on feedback from the previously opened PR, the styles for standalone lessons have also been adjusted to align with the design.

@renintw renintw force-pushed the 2869-add-facilitator-note branch from 482c918 to 1e2d4b0 Compare December 5, 2024 13:26
@renintw renintw self-assigned this Dec 10, 2024
@renintw renintw marked this pull request as ready for review December 10, 2024 08:54
@renintw renintw mentioned this pull request Dec 10, 2024
@renintw renintw added [Component] Learn Theme Website development issues related to the Learn theme. [Component] Sensei Website development issues related to the Sensei plugin installed on Learn. labels Dec 10, 2024
@renintw renintw added this to the Learning Pathways Post-launch milestone Dec 10, 2024
@renintw renintw requested a review from ryelle December 10, 2024 08:57
@renintw renintw changed the title 2869 add facilitator note Add facilitator note Dec 10, 2024
@fcoveram
Copy link

All looks good to me. The only remaining point is on opening the facilitator notes. @ryelle shared a clear response for my question about opening a new tab or not

Is there progress that a user can loose? Lessons are mostly articles, as I recall from past testing. Clicking a link and then using the back button wouldn't cause "lose of data" when reading an article. Would this show up on a quiz, or are there many interactive lessons?

If there are, and this would be a "lose of data" case, the G200 technique also recommends giving advance warning that this will be a new tab (typically this is done with the ↗️ icon, but we're already using that for external links). The indicator would need to be visible and screen-reader-accessible.

Otherwise, I think it would be fine to keep it as a new page and let users open it as they choose.

As per my understanding, lessons do not display content that could be loose, as in quizzes. Therefore, we should open the notes by refreshing the page, as with the default page browsing.

@jonathanbossenger
Copy link
Collaborator

Hi folks, just a note on the conversation between opening in a new tab vs refreshing the page.

As per my understanding, lessons do not display content that could be loose, as in quizzes. Therefore, we should open the notes by refreshing the page, as with the default page browsing.

I want to remind everyone of the purpose of the facilitator notes/lesson plans. These are typically used by teachers/educators/presenters, not for learners. While learners might decide to view this content, they are not the primary consumers of lesson plan content. (lesson != lesson plan)

As a teacher/educator/presenter at a meetup, I want to present a topic based on a lesson on learn.wordpress.org. I open the lesson, and I want to see the facilitator's notes that help me prepare for it. I would, therefore, want to be able to see both at the same time, either in the modal we originally suggested or in a separate tab. (My personal preference would be to drag the tabs side by side, but that's me.)

So, this is not about learners losing content/progress but about making it easier for facilitators to prepare. Opening this in a new tab would still be ideal.

@jonathanbossenger
Copy link
Collaborator

@renintw I've tested this on my local environment, and it all works as expected. One thing I did note is that on higher resolution screens (I'm currently on 2560 x 1440 DPIx2), the facilitator notes link appears in a weird position (screenshot below)

Lesson-1-–-Lesson-Learn-WordPress-org-12-10-2024_12_36_PM

@kaitohm as this link between lessons and lesson plans has been implemented as a block in the editor, we might want to decide if we want the block to be added at the top or the bottom of the lesson. I'd recommend we require it at the bottom.

@ryelle
Copy link
Contributor

ryelle commented Dec 10, 2024

As a teacher/educator/presenter at a meetup, I want to present a topic based on a lesson on learn.wordpress.org. I open the lesson, and I want to see the facilitator's notes that help me prepare for it. I would, therefore, want to be able to see both at the same time, either in the modal we originally suggested or in a separate tab. (My personal preference would be to drag the tabs side by side, but that's me.)

So, this is not about learners losing content/progress but about making it easier for facilitators to prepare. Opening this in a new tab would still be ideal.

It's not about preventing opening in a new tab, so users can still cmd+click or right-click to open in a new tab. The issue is that it's an accessibility anti-pattern to force opening in a new tab. The exception would be if there was data loss, which is why that conversation came up. Leaving the option to the user is the best approach.

@jonathanbossenger
Copy link
Collaborator

@ryelle ah, my apologies, I understand now. When I originally read the conversation around when it was decided to move away from a modal to the external link, I had understood it would open in a new tab by default. I do understand that it's an accessibility anti-pattern to force opening in a new tab. I wasn't aware of the exception around data loss, which is why I misread that discussion.

@kaitohm I know the facilitator notes as a modal was something that @Piyopiyo-Kitsune originally wanted, specifically to be able to see both at the same time. Do you foresee any issues with the facilitator notes opening in the current window, instead of a new tab, and folks rather choosing, through the use of cmd/ctrl + click or right click open in a new tab?

@kaitohm
Copy link
Contributor

kaitohm commented Dec 11, 2024

@renintw Thanks for working on this! It's exciting to see this moving forward.

@jonathanbossenger

I'd recommend we require it at the bottom.

I agree. We can require as such in the team documentation when that's updated.

Do you foresee any issues with the facilitator notes opening in the current window, instead of a new tab, and folks rather choosing, through the use of cmd/ctrl + click or right click open in a new tab?

Not really...? I think we could implement it that way, and then iterate further on if we get feedback.

To clarify, we're not using a modal to show this information? It would replace the content on the current page?

@fcoveram
Copy link

Thanks @ryelle for the clarification.

To clarify, we're not using a modal to show this information? It would replace the content on the current page?

Yes. We're moving away from using the modal, as suggested here (a closed ticket that implemented the modal), and the link will refresh the page. As pointed out here, we're not forcing users to open a new tab and they decide how to navigate.

@renintw
Copy link
Contributor Author

renintw commented Dec 11, 2024

@renintw I've tested this on my local environment, and it all works as expected. One thing I did note is that on higher resolution screens (I'm currently on 2560 x 1440 DPIx2), the facilitator notes link appears in a weird position (screenshot below)

Oh, I figured out the reason. It's because I inserted the facilitator notes after the course progress counter and assumed that non-standalone courses would always have a course progress counter. I overlooked the view seen by unenrolled students.

It’s been fixed.

Do you think we'd want to restrict who can see this label? @jonathanbossenger

--

Note: I’ve updated the link to open in the same tab instead of a new one.

@jonathanbossenger
Copy link
Collaborator

Do you think we'd want to restrict who can see this label? @jonathanbossenger

Hmm good question. The person who would use this would be someone who is preparing an in-person lesson somewhere, based on this content. They might be enrolled in the course, or they might not. Therefore I don't think we'd want to restrict who can see this label.

@renintw renintw force-pushed the 2869-add-facilitator-note branch from 458d442 to affb77e Compare December 11, 2024 12:57
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

A Lesson Facilitator Note block has been added, which can be used within the Lesson Editor to select the corresponding lesson plan. The content can be edited and saved directly within the editor.

It surprised me that this content is editable and saves back to the Lesson Plan. Once it's in the Lesson editor, the way it's framed, I expected it to disconnect from the original. Was it being directly editable a feature requirement?

Will there be a case where someone wants to add facilitator notes to a lesson, but a separate Lesson Plan doesn't exist?


const fetchLessonPlans = debounce( ( searchTerm ) => {
apiFetch( {
path: `/wp/v2/lesson-plan?search=${ encodeURIComponent( searchTerm ) }&per_page=10`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a lot of trouble finding specific lesson plans in my local site - I was typing exact keywords in the lesson title, but nothing would show up. I think because this search is default WP search, so it's returning when the keywords match in title or content, but the combobox control only matches results on title.

So the API is limited to 10 items, and my search "block directory" returned more than 10 items, pushing the page I wanted out of the options. And nothing would show up when I typed, because "block directory" is not a match in any of the results I had:

  • Create a Basic Child Theme for Block Themes
  • WebP images in WordPress
  • Υποβολή Μοτίβων μπλοκ (Block Patterns) στον κατάλογο.
  • Πώς να χρησιμοποιήσετε και να συνεισφέρετε στον κατάλογο φωτογραφιών του WordPress αλλά και στο Openverse
  • Submitting Block Patterns to the Directory
  • Template Editor
  • How to Use and Contribute to the WordPress Photo Directory and Openverse
  • How to Build Low-Code Block Patterns
  • How to Create and Register a Block Pattern
  • Difference between Reusable Blocks, Block Pattern, Templates, Template Parts

Maybe increase the result number? Or see if you can adjust the search for this endpoint to only do title searches.

You could preload the ID/title pairs, and not use an API request for this at all. With 150 published lesson plans, it shouldn't be that slow. Or if you'd rather not do that on the server side, fire off an API request to get all the lesson plans when the block loads (instead of just if ( lessonPlanId )).

@renintw
Copy link
Contributor Author

renintw commented Dec 13, 2024

It surprised me that this content is editable and saves back to the Lesson Plan. Once it's in the Lesson editor, the way it's framed, I expected it to disconnect from the original. Was it being directly editable a feature requirement?

Will there be a case where someone wants to add facilitator notes to a lesson, but a separate Lesson Plan doesn't exist?

I inferred this from this comment and the comments from the PR. However after taking a closer look, it seems that the main intention is to avoid editing the same document in different places.

So I think I can also change it here so that editing is not possible, or even hide the content entirely. And instead, provide a link to edit or add a new lesson plan.

What do you think, does it provide any benefit to edit directly within the lesson? @jonathanbossenger @kaitohm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Learn Theme Website development issues related to the Learn theme. [Component] Sensei Website development issues related to the Sensei plugin installed on Learn.
Projects
Status: 👀 In review (PRs only)
Development

Successfully merging this pull request may close these issues.

A revamped design in Lesson flow
5 participants