feat(ol_openedx_feedback): send a visible title only for blocks that render one

src/ol_openedx_feedback/README.rst

@@ -1,72 +1,88 @@
 ol-openedx-feedback
 ###################
 
-An Open edX plugin that adds a per-block "Send feedback" trigger to applicable
-leaf blocks in the LMS via an ``XBlockAside``.  The trigger is shown only to
-authenticated learners (never in Studio author/preview mode and never to
-anonymous users).
-
-When a learner clicks the trigger the aside posts an ``ol-feedback::drawer-open``
-message to its parent window (the Learning MFE) using ``window.parent.postMessage``.
-The message payload carries the block context needed to identify the content
-being rated:
-
-- ``courseId`` — the course key
-- ``blockUsageKey`` — the block's usage key
-- ``blockType`` — the XBlock category (e.g. ``problem``, ``video``)
-- ``blockDisplayName`` — the block's display name
-
-The Learning MFE receives the message and opens the feedback drawer, which
-submits the learner's feedback directly to the **mit-learn** service.  This
-plugin does **not** persist anything in edx-platform and exposes no REST API.
+An Open edX plugin that adds a "Send feedback" megaphone to course blocks in the
+LMS via an ``XBlockAside``. Clicking it opens the feedback drawer in the Learning
+MFE, which submits the learner's feedback to the **mit-learn** service. This
+plugin renders the trigger and nothing else: no models, no REST API, nothing
+persisted in edx-platform.
+
+The trigger renders only for authenticated learners — never in Studio
+author/preview mode, never for anonymous users. It sits right-aligned below the
+block, or docked beside the AskTIM button on blocks that have one.
+
+Messages
+========
+
+The aside talks to its parent window (the Learning MFE) with
+``window.parent.postMessage``, always addressed to
+``settings.LEARNING_MICROFRONTEND_URL`` and never to ``"*"``.
+
+Sent on click, as ``ol-feedback::drawer-open``:
+
+- ``payload.courseId`` — the course key
+- ``payload.blockUsageKey`` — the block's usage key
+- ``payload.blockType`` — the XBlock category (e.g. ``video``, ``problem``)
+- ``payload.blockDisplayName`` — the Studio ``display_name``. Recorded on the
+  submission so the content team can find the component; the drawer doesn't
+  display it.
+- ``payload.visibleTitle`` — the title the learner can see on the page, or ``""``
+  when this plugin can't confirm there is one. This is the only name the drawer
+  displays; see `Visible titles`_.
+- ``viaKeyboard`` — true when the click came from the keyboard. The drawer
+  renders cross-origin and so can't use ``:focus-visible``; it uses this to
+  decide whether to highlight its heading.
+
+Received from the MFE, both so keyboard focus can return to the megaphone
+(WCAG 2.4.3):
+
+- ``ol-feedback::drawer-closed`` — the drawer has closed; focus the trigger that
+  opened it.
+- ``ol-feedback::focus-trigger`` — the drawer's "return to block" link; focus the
+  trigger but leave the drawer open.
 
 Version Compatibility
-======================
+=====================
 
 For this plugin's compatibility with Open edX, see the
 `Open edX Release Compatibility table <../../docs#open-edx-release-compatibility>`_.
 
 Installation
 ============
 
-Install the package into the LMS Python environment:
+1. Install the package into the LMS Python environment and restart the LMS:
 
-.. code-block:: bash
+   .. code-block:: bash
 
-    pip install ol-openedx-feedback
+       pip install ol-openedx-feedback
 
-The plugin registers itself automatically through its entry points — the
-``xblock_asides.v1`` aside plus the ``lms.djangoapp`` app config — so no changes
-to ``INSTALLED_APPS`` are required. Restart the LMS after installing. (The
-trigger is learner-facing only, so the plugin is LMS-only and is not installed
-in Studio/CMS.)
+   The plugin registers itself through its entry points — the ``xblock_asides.v1``
+   aside plus the ``lms.djangoapp`` app config — so ``INSTALLED_APPS`` needs no
+   change. The trigger is learner-facing only, so the plugin is LMS-only and is
+   not installed in Studio/CMS.
 
-Enable XBlock asides in the LMS admin
--------------------------------------
+2. **Enable XBlock asides.** No aside renders until they are switched on. In the
+   LMS admin, open **XBlock Asides Config**
+   (``/admin/lms_xblock/xblockasidesconfig/``), add an entry, and check
+   **Enabled**. Keep the block types you want the trigger on out of **Disabled
+   blocks** (space-separated; defaults to ``about course_info static_tab``).
 
-XBlock asides must be turned on for any aside (including this one) to render.
-In the LMS Django admin, open **XBlock Asides Config**
-(``/admin/lms_xblock/xblockasidesconfig/``), add a new entry, and check
-**Enabled**. Make sure the block types you want the feedback trigger on are
-**not** listed in **Disabled blocks** (the space-separated field defaults to
-``about course_info static_tab``).
-
-Enablement
-==========
-
-Feedback is gated by the ``ol_openedx_feedback.feedback_enabled`` course waffle
-flag (default off). Enable it for the desired courses (or globally) to roll out.
+3. **Turn on the waffle flag.** The trigger is gated per course by the
+   ``ol_openedx_feedback.feedback_enabled`` course waffle flag, off by default.
+   Enable it for the courses you're rolling out to, or globally.
 
 Configuration
 =============
 
+Both settings below are read from ``ENV_TOKENS`` (e.g. in ``lms.yml``) by the
+plugin's ``settings.common`` hook and exposed as Django settings of the same
+name.
+
 Excluded block types
 --------------------
 
-By default the trigger renders on every leaf block and is suppressed only on
-structural containers (``course`` / ``chapter`` / ``sequential`` / ``vertical``).
-To additionally exclude one or more block types (for example ``html``), override
-the excluded set through ``ENV_TOKENS`` (e.g. in ``lms.yml``):
+The trigger renders on every block type except a set of structural containers.
+Override the set to exclude more types — for example ``html``:
 
 .. code-block:: yaml
 
@@ -77,19 +93,40 @@ the excluded set through ``ENV_TOKENS`` (e.g. in ``lms.yml``):
       - vertical
       - html
 
-The plugin reads this value via its ``settings.common`` ``plugin_settings`` hook
-and exposes it as the ``OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES`` Django
-setting, defaulting to the structural set above when unset.
+Unset, it defaults to ``course``, ``chapter``, ``sequential`` and ``vertical``.
 
 Text label
 ----------
 
-The trigger shows only the megaphone icon by default. To also render the
-"Feedback" text label beside it, enable it through ``ENV_TOKENS``:
+The trigger is icon-only by default. To render the "Feedback" text label beside
+the megaphone:
 
 .. code-block:: yaml
 
     OL_OPENEDX_FEEDBACK_SHOW_LABEL: true
 
-This is exposed as the ``OL_OPENEDX_FEEDBACK_SHOW_LABEL`` Django setting via the
-same ``plugin_settings`` hook, defaulting to ``false`` (icon-only) when unset.
+Visible titles
+==============
+
+For most block types ``display_name`` is an authoring label that never appears
+on the page ("Wk3 intro copy - REVISED, do not reuse"), so showing it in the
+feedback drawer would confuse the learner. The plugin therefore keeps a curated
+map of the types that *do* render a title, and the field holding it —
+``display_name`` for most, ``title`` for ``openassessment``, ``block_name`` for
+``survey``. Types outside the map send an empty ``visibleTitle``, which the
+drawer renders as generic block-type wording.
+
+Two details:
+
+- A type whose title is author-toggleable (``drag-and-drop-v2``'s ``show_title``)
+  reports its title only when the toggle is on.
+- Keys are usage-key block types (``category``). The extracted
+  ``xblocks-contrib`` variants register under ``_<name>_extracted`` entry points
+  but keep the plain category, so they need no separate entry.
+
+**The map is not a setting.** Each entry asserts that the block type draws that
+field on the learner's page, and the value goes verbatim into the drawer's
+question text — so a wrong entry leaks the author's private note, which is the
+bug the map exists to prevent. Changing it means editing
+``DEFAULT_VISIBLE_TITLE_FIELDS`` in ``constants.py`` and having the claim
+reviewed. Leaving a type out only costs generic wording.

src/ol_openedx_feedback/ol_openedx_feedback/block.py

@@ -20,7 +20,7 @@
 
 from ol_openedx_feedback.compat import get_feedback_enabled_flag
 from ol_openedx_feedback.constants import DEFAULT_SHOW_LABEL
-from ol_openedx_feedback.utils import is_aside_applicable_to_block
+from ol_openedx_feedback.utils import get_visible_title, is_aside_applicable_to_block
 
 log = logging.getLogger(__name__)
 
@@ -79,7 +79,10 @@ def student_view_aside(self, block, context=None):  # noqa: ARG002
                     "courseId": str(block_usage_key.course_key),
                     "blockUsageKey": str(block_usage_key),
                     "blockType": block_type,
+                    # Studio name: recorded on the submission, never shown to
+                    # the learner. Only visibleTitle reaches the panel.
                     "blockDisplayName": block.display_name or "",
+                    "visibleTitle": get_visible_title(block),
                 },
             },
         )

src/ol_openedx_feedback/ol_openedx_feedback/constants.py

@@ -6,5 +6,29 @@
 # exclude a content type like ``html``).
 DEFAULT_EXCLUDED_BLOCK_TYPES = {"course", "chapter", "sequential", "vertical"}
 
+# Block types that render a title the learner can see, mapped to the field
+# holding it. An unlisted type sends no title and the panel names the type
+# instead. A wrong entry leaks an author-only Studio name, so when in doubt
+# leave it out — see "Visible titles" in the README before editing.
+DEFAULT_VISIBLE_TITLE_FIELDS = {
+    "annotatable": "display_name",
+    "drag-and-drop-v2": "display_name",
+    "edx_sga": "display_name",
+    "lti": "display_name",
+    "lti_consumer": "display_name",
+    "pdf": "display_name",
+    "poll": "display_name",
+    "problem": "display_name",
+    "staffgradedxblock": "display_name",
+    "video": "display_name",
+    "videoalpha": "display_name",
+    "word_cloud": "display_name",
+    "openassessment": "title",
+    "survey": "block_name",
+}
+
+# Types whose title renders only when an author-controlled boolean is on.
+TITLE_VISIBILITY_TOGGLES = {"drag-and-drop-v2": "show_title"}
+
 # Show the "Feedback" text label next to the icon. Off by default (icon only).
 DEFAULT_SHOW_LABEL = False

src/ol_openedx_feedback/ol_openedx_feedback/utils.py

@@ -1,8 +1,13 @@
 """Utility helpers for ol_openedx_feedback."""
 
 from django.conf import settings
+from django.utils.functional import Promise
 
-from ol_openedx_feedback.constants import DEFAULT_EXCLUDED_BLOCK_TYPES
+from ol_openedx_feedback.constants import (
+    DEFAULT_EXCLUDED_BLOCK_TYPES,
+    DEFAULT_VISIBLE_TITLE_FIELDS,
+    TITLE_VISIBILITY_TOGGLES,
+)
 
 
 def get_excluded_block_types():
@@ -24,3 +29,23 @@ def is_aside_applicable_to_block(block):
     """Feedback applies to every block type except excluded containers."""
     block_type = getattr(block, "category", None)
     return bool(block_type) and block_type not in get_excluded_block_types()
+
+
+def get_visible_title(block):
+    """Return the title the learner can see on the page, or "" if there is none.
+
+    "" tells the panel to name the block type instead.
+    """
+    block_type = getattr(block, "category", None)
+    title_field = DEFAULT_VISIBLE_TITLE_FIELDS.get(block_type)
+    if not title_field:
+        return ""
+
+    toggle_field = TITLE_VISIBILITY_TOGGLES.get(block_type)
+    if toggle_field and not getattr(block, toggle_field, False):
+        return ""
+
+    title = getattr(block, title_field, "")
+    if not isinstance(title, str | Promise):
+        return ""
+    return str(title).strip()

src/ol_openedx_feedback/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "ol-openedx-feedback"
-version = "0.3.0"
+version = "0.3.1"
 description = "An Open edX plugin to collect per-block learner feedback"
 authors = [{name = "MIT Office of Digital Learning"}]
 license = "BSD-3-Clause"

src/ol_openedx_feedback/tests/test_aside.py

@@ -66,6 +66,34 @@ def test_trigger_is_not_marked_as_a_dialog_popup(self):
         assert "aria-haspopup" not in fragment.content
         assert 'aria-label="' in fragment.content
 
+    @skip_unless_lms
+    def test_payload_sends_visible_title_for_a_block_that_renders_it(self):
+        """A video draws its name as a heading, so the panel may use it."""
+        self.runtime.user_id = 5
+        self.runtime.is_author_mode = False
+        self.video_aside_instance.runtime = self.runtime
+
+        fragment = self.video_aside_instance.student_view_aside(self.video_block)
+        payload = fragment.json_init_args["drawer_payload"]
+
+        assert payload["visibleTitle"] == "My Video"
+        assert payload["blockDisplayName"] == "My Video"
+
+    @skip_unless_lms
+    def test_payload_withholds_studio_name_for_a_block_that_hides_it(self):
+        """An html block renders only its body, so its Studio name is
+        author-only: withheld from the panel, still recorded.
+        """
+        self.runtime.user_id = 5
+        self.runtime.is_author_mode = False
+        self.html_aside_instance.runtime = self.runtime
+
+        fragment = self.html_aside_instance.student_view_aside(self.html_block)
+        payload = fragment.json_init_args["drawer_payload"]
+
+        assert payload["visibleTitle"] == ""
+        assert payload["blockDisplayName"] == "An HTML Block"
+
     @data(
         *[
             ["video", True, False, True],