-
Notifications
You must be signed in to change notification settings - Fork 107
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
base: trunk
Are you sure you want to change the base?
Add facilitator note #3052
Conversation
482c918
to
1e2d4b0
Compare
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
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. |
Hi folks, just a note on the conversation between opening in a new tab vs refreshing the page.
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. |
@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) @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. |
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. |
@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? |
@renintw Thanks for working on this! It's exciting to see this moving forward.
I agree. We can require as such in the team documentation when that's updated.
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? |
Thanks @ryelle for the clarification.
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. |
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. |
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. |
458d442
to
affb77e
Compare
There was a problem hiding this 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`, |
There was a problem hiding this comment.
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 )
).
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 |
Just to pause a minute, what we're seeing proposed here is very different from the feature requirements Training had requested. To bring back this comment, we're looking for:
It seems none of these requests are being met with the current implementation suggestion. Could we clarify the rationale behind why each of these three requests are not being implemented? I'd like to make sure the Training team understands the reasons for course correction here before we sign off on the suggested implementation. @renintw @ryelle @fcoveram 🙂 (This comment was referenced as the reason for moving away from a modal. But the reason behind the decision isn't clear to me there, only that the decision to move away was made in that comment.) |
I can speak for the first point. Let me recap how the design unrolled. The first proposal explored the idea of displaying the notes in a sidebar toggled from a link in the page header. Then, the feedback received was about concerns over showing extensive content in a narrow area. The second iteration shifted to show the notes in a popover and modal, but a personal confusion on the content extension did not suit the real use case attached in this comment. From there, I suggested moving us away from the popover and modal approach and instead going with a full-page solution. In that comment, I also suggested opening the notes in a new tab, but since the accessibility criteria (G200) says to apply “only when necessary,” I asked for thoughts. This comment elaborated on the “lose of data” scenario and a possible solution (adding a top-right icon) from the G200. However, the content does not belong to any of the two situations described in the accessibility technique. With the above, the comments on this PR (from here) have been mainly about applying G200 and “not forcing” users to open a new tab. Instead, they browse as within their preferences. That includes indeed opening a new tab. To me, the following description from the accessibility guidelines criteria summarizes the core idea of not forcing users to open tabs.
Given the above, I would follow the accessibility recommendation and not open the notes in new tab. Looking forward to seeing the Training team's thoughts and who makes the decision, as this seems to be our current blocker. |
@fcoveram I wonder if it would be useful to discuss your concern about a long modal being frustrating. Are you concerned that the users of facilitator notes would find it frustrating having to scroll for the content they need in the notes or something else? |
The scroll interaction is not a problem by itself; it’s an accepted behavior on many devices. If most facilitator notes are long and have rich text content that might be difficult to read in a narrow space, like headings and bullets that add indentation spaces, then I’m drawn to display them on a different page. You mentioned something similar in a previous comment.
The context is also crucial. And it seems there is no need for having them side by side, as pointed out in this comment.
(Notes in bold were added by me) |
Thank you for the detailed summary, @fcoveram , this really helps 😃 Have we considered a full-screen modal instead of a new page? Here's an example of what I'm talking about. https://www.w3schools.com/bootstrap5/tryit.asp?filename=trybs_modal_fullscreen&stacked=h (I've also seen modals like this with a zoom in/out animation on opening/closing.) From a content maintenance point of view, the team would really like to get rid of the additional post type. Having lesson content and facilitator notes live in different locations requires additional manual tracking to link the two and update the other when one has been modified. I'd say if we can store all information in a single post type, then the visual representation is less of an issue here. (Though, I think a modal of some sorts would provide a stronger sense of continuity between the content and facilitator notes.) I've brought the Training Team's attention to the last few comments here so that the team is aware of the progress of this discussion: https://wordpress.slack.com/archives/C02RW657Q/p1735020507769879 |
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
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.