feat: Add formatting buttons to new Markdown editor #2073 #2456

ahtesham-quraish · 2025-09-17T07:14:15Z

Description

Add formatting buttons to new Markdown editor #2073

Supporting information

codecov · 2025-09-17T07:28:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.65%. Comparing base (ab645ad) to head (e8754a6).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2456   +/-   ##
=======================================
  Coverage   94.64%   94.65%           
=======================================
  Files        1187     1189    +2     
  Lines       26220    26306   +86     
  Branches     5839     5854   +15     
=======================================
+ Hits        24817    24901   +84     
- Misses       1333     1335    +2     
  Partials       70       70

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jacobo-dominguez-wgu

Great work! it looks good in general, just a few comments.

jacobo-dominguez-wgu · 2025-09-17T18:36:37Z

src/editors/sharedComponents/CodeEditor/EditorToolbar.jsx

+
+const EditorToolbar = ({ editorRef }) => {
+  const intl = useIntl();
+  const cm = editorRef;


I would appreciate a better self described name for this variable, or maybe we can use the prop variable instead of this new one.

jacobo-dominguez-wgu · 2025-09-17T18:40:59Z

src/editors/sharedComponents/CodeEditor/hooks.js

    const view = new EditorView({ state: newState, parent: ref.current });
    // eslint-disable-next-line no-param-reassign
    upstreamRef.current = view;
+    if (onReady) {


It can be simplified with the optional chaining operator like this: onReady?.(view)

yeah that make sense

jacobo-dominguez-wgu · 2025-09-17T18:43:14Z

src/editors/sharedComponents/CodeEditor/EditorToolbar.jsx

@@ -0,0 +1,79 @@
+import React from 'react';
+import { Button, Icon } from '@openedx/paragon';
+import PropTypes from 'prop-types';


Not mandatory but it would be nice to start using typescript for new components, that way we can get rid of the PropTypes.

jacobo-dominguez-wgu · 2025-09-17T18:45:42Z

src/editors/sharedComponents/CodeEditor/EditorToolbar.scss

+  align-items: center;
+  gap: 4px;
+  padding: 6px;
+  background: #D1DAE5;


Just wondering if we can use design tokens here instead of the raw values on this file.

I did not get it. can you please give me example?

Here is the pr on which design tokens were added to the repo #2187, it is basically to use predefined variables instead of hardcoded values, but is just a nice to have.

https://paragon-openedx.netlify.app/foundations/colors/

open-craft-grove · 2025-09-25T18:00:07Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

brian-smith-tcril · 2025-09-25T20:12:53Z

The main thing that concerns me about this PR is the amount of custom CSS being used. The more custom CSS in an MFE, the harder it is for theme authors to verify their theme works throughout the platform.

Custom CSS also concerns me from an a11y stand point. Paragon components and styles have been reviewed for accessibility, so anything custom outside of that would need to be reviewed for a11y as well.

I pulled this down locally to try to get a sense of how possible it would be to reduce the amount of custom CSS needed to accomplish this.

My first attempt was using a Paragon ButtonToolbar. By using ButtonGroups with size set to sm I was able to make some things look decent. With no custom CSS some of it managed to look pretty good.

<ButtonToolbar>
  <ButtonGroup size='sm'>
        [...]
        <Button variant="light" iconBefore={CheckBoxIcon} onClick={() => insertAtCursor(CHECKBOXES)}>
          {intl.formatMessage(messages.editorToolbarCheckboxButtonLabel)}
        </Button>
        [...]
      </ButtonGroup>
  </ButtonGroup>
  [...]
</ButtonToolbar>

I ran into issues with it being too big (either the buttons overflowed to the next line, or if I prevented overflowing to the next line some of the buttons had 2 lines of text so everything looked really tall.

Looking for something smaller, I tried using Paragon Chips

    <Stack direction="horizontal" className="bg-primary-300">
      <Chip variant="dark" onClick={() => insertAtCursor(HEADING)}>
        <span>{'<H>'}</span>{' '}{intl.formatMessage(messages.editorToolbarHeadingButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={ViewList} onClick={() => insertAtCursor(MULTIPLE_CHOICE)}>
        {intl.formatMessage(messages.editorToolbarMultipleChoiceButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={CheckBoxIcon} onClick={() => insertAtCursor(CHECKBOXES)}>
        {intl.formatMessage(messages.editorToolbarCheckboxButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={Abc} onClick={() => insertAtCursor(TEXT_INPUT)}>
        {intl.formatMessage(messages.editorToolbarTextButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={Pgn123} onClick={() => insertAtCursor(TEXT_INPUT)}>
        {intl.formatMessage(messages.editorToolbarNumericalButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={ArrowDropDown} onClick={() => insertAtCursor(DROPDOWN)}>
        {intl.formatMessage(messages.editorToolbarDropdownButtonLabel)}
      </Chip>

      <Chip variant="dark" iconBefore={Lightbulb} onClick={() => insertAtCursor(EXPLANATION)}>
        {intl.formatMessage(messages.editorToolbarExplanationButtonLabel)}
      </Chip>
    </Stack>

but even that didn't fit everything

The next thing I tried was just minimizing the amount of custom CSS, and only referencing CSS vars instead of any hardcoded values.

I was able to get to this

with

EditorToolbar.tsx

    <ButtonToolbar className="editor-toolbar">
      <ButtonGroup size="sm">
        <Button variant="light" onClick={() => insertAtCursor(HEADING)}>
          <span className="btn-icon-before font-weight-bold">{'<H>'}</span>
          {intl.formatMessage(messages.editorToolbarHeadingButtonLabel)}
        </Button>

        <Button variant="light" iconBefore={ViewList} onClick={() => insertAtCursor(MULTIPLE_CHOICE)}>
          {intl.formatMessage(messages.editorToolbarMultipleChoiceButtonLabel)}
        </Button>

        <Button variant="light" iconBefore={CheckBoxIcon} onClick={() => insertAtCursor(CHECKBOXES)}>
          {intl.formatMessage(messages.editorToolbarCheckboxButtonLabel)}
        </Button>
      </ButtonGroup>
      <ButtonGroup size="sm">

        <Button variant="light" iconBefore={Abc} onClick={() => insertAtCursor(TEXT_INPUT)}>
          {intl.formatMessage(messages.editorToolbarTextButtonLabel)}
        </Button>

        <Button variant="light" iconBefore={Pgn123} onClick={() => insertAtCursor(NUMERICAL_INPUT)}>
          {intl.formatMessage(messages.editorToolbarNumericalButtonLabel)}
        </Button>

        <Button
          variant="light"
          iconBefore={ArrowDropDown}
          onClick={() => insertAtCursor(DROPDOWN)}
        >
          {intl.formatMessage(messages.editorToolbarDropdownButtonLabel)}
        </Button>
      </ButtonGroup>

      <ButtonGroup size="sm">
        <Button variant="light" iconBefore={Lightbulb} onClick={() => insertAtCursor(EXPLANATION)}>
          {intl.formatMessage(messages.editorToolbarExplanationButtonLabel)}
        </Button>
      </ButtonGroup>
    </ButtonToolbar>

EditorToolbar.scss

.editor-toolbar {
  background-color: var(--pgn-color-btn-bg-light);

  .btn {
    font-size: var(--pgn-typography-font-size-micro);

    .btn-icon-before {
      width: var(--pgn-size-icon-xs);
      height: var(--pgn-size-icon-xs);
    }
  }
}

Which doesn't match the implementation in this PR 1:1 but is the closest of the things I tried. Also, since the goal of this was just to get a proof-of-concept implementation that looks similar to what's in this PR, I am still concerned about the a11y implications of it.

Before this lands, I'd like to see if @kblemel has thoughts about this from an a11y perspective. Then those a11y requirements can be used to inform the final design, and I'll be able to provide suggestions and another review to ensure the implementation utilizes Paragon components, utility classes, and CSS variables.

kdmccormick · 2025-09-25T20:19:35Z

Thanks for that feedback @brian-smith-tcril ! @kblemel and @sdaitzman , any feedback you provide is also appreciated.

I want to make sure that we land this before the Ulmo cutoff (Oct 9)--if there are still improvements to be made after that, we can certainly fix forward and/or backport.

@ahtesham-quraish Would you be able to factor in Brian's feedback?

kdmccormick

@ahtesham-quraish , could you factor in Brian's feedback?

ahtesham-quraish force-pushed the ahtesham/#2073 branch 2 times, most recently from 93664e3 to 5f1f774 Compare September 17, 2025 07:17

jacobo-dominguez-wgu reviewed Sep 17, 2025

View reviewed changes

ahtesham-quraish mentioned this pull request Sep 19, 2025

Add formatting buttons to new Markdown editor #2073

Open

feat: Add formatting buttons to new Markdown editor openedx#2073

e8754a6

ahtesham-quraish force-pushed the ahtesham/#2073 branch from 5f1f774 to e8754a6 Compare September 22, 2025 07:46

kdmccormick self-requested a review September 25, 2025 15:02

kdmccormick added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Sep 25, 2025

kdmccormick requested changes Oct 6, 2025

View reviewed changes

feat: Add formatting buttons to new Markdown editor #2073 #2456

Are you sure you want to change the base?

feat: Add formatting buttons to new Markdown editor #2073 #2456

Conversation

ahtesham-quraish commented Sep 17, 2025

Description

Supporting information

Uh oh!

codecov bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jacobo-dominguez-wgu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

open-craft-grove commented Sep 25, 2025

Uh oh!

brian-smith-tcril commented Sep 25, 2025

Uh oh!

kdmccormick commented Sep 25, 2025

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Sep 17, 2025 •

edited

Loading