feat: enable headings to be configurable by ShugKnight24 · Pull Request #754 · basecamp/lexxy

ShugKnight24 · 2026-02-26T07:44:21Z

Hey there, listened to a chat between @dhh, @ThePrimeagen, and @tjdevries. I really enjoyed and I came away with a lot of value and wanted to pay it forward

This PR allows headings to be configured and closes #614

Based on feedback from @samuelpecher, I removed the UI updates and only kept headings being configurable

Cheers!

Preview

configurable_headings_rotate.mov

Tests

- Closes basecamp#614

Copilot

Pull request overview

This pull request implements configurable headings with a dropdown selector, replacing the previous rotation-based heading selection. The change addresses issue #614 by allowing users to select specific heading levels (H1-H6) from a dropdown and making the available heading levels configurable.

Changes:

Replaced rotation-based heading button with a dropdown selector showing all configured heading levels
Added configuration option for customizing available heading levels (defaults to H1-H6)
Added visual indication of currently selected heading level in the dropdown
Introduced setHeading command to set specific heading levels directly

Reviewed changes

Copilot reviewed 10 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/system/toolbar_test.rb	Updated tests to use dropdown selection instead of rotation, added tests for active state highlighting
test/javascript/editor/headings_configuration.test.js	New test file for heading configuration behavior
src/elements/dropdown/heading.js	New dropdown component implementing heading selection UI
src/elements/index.js	Registered new HeadingDropdown element
src/elements/toolbar.js	Replaced heading button with dropdown structure (includes formatting changes)
src/editor/selection.js	Added headingTag to selection format for active state detection (includes formatting changes)
src/editor/command_dispatcher.js	Added dispatchSetHeading method and updated dispatchRotateHeadingFormat to respect configuration
src/config/lexxy.js	Added default headings configuration
app/assets/stylesheets/lexxy-editor.css	Added styles for heading dropdown (includes trailing whitespace cleanup)
app/assets/javascript/lexxy.js	Compiled version of source changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/system/toolbar_test.rb

test/javascript/editor/headings_configuration.test.js

src/elements/dropdown/heading.js

test/javascript/editor/headings_configuration.test.js

- top level configure - test h1 - test empty case - validate headings to prevent other elements from being used - revert previous whitespace changes

ShugKnight24 · 2026-02-26T08:19:43Z

src/elements/toolbar.js

+    this.#dropdowns.forEach((details) => {
+      details.open = false
+    })
+  }


this indentation change looks correct

Copilot

Pull request overview

Copilot reviewed 10 out of 15 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/elements/toolbar.js:105

Indentation in #dispatchButtonCommand looks off (the dispatchCommand line is indented an extra level), which makes this block harder to read and inconsistent with the rest of the file. Can you reformat this block to match the surrounding indentation?

  #dispatchButtonCommand(event, { dataset: { command, payload } }) {
    const isKeyboard = event instanceof PointerEvent && event.pointerId === -1

    this.editor.update(() => {
      this.editor.dispatchCommand(command, payload)
    }, { tag: isKeyboard ? SKIP_DOM_SELECTION_TAG : undefined })
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/elements/dropdown/heading.js

app/assets/stylesheets/lexxy-editor.css

test/javascript/editor/headings_configuration.test.js

src/editor/command_dispatcher.js

Copilot · 2026-02-26T08:22:45Z

src/editor/command_dispatcher.js

+  dispatchSetHeading(tag) {
+    const selection = $getSelection()
+    if (!$isRangeSelection(selection)) return
+
+    if (!tag) {
+      this.contents.removeFormattingFromSelectedLines()
+      return
+    }
+
+    if ($isRootOrShadowRoot(selection.anchor.getNode())) {
+      selection.insertNodes([ $createHeadingNode(tag) ])
+      return
+    }
+
+    this.contents.insertNodeWrappingEachSelectedLine(() => $createHeadingNode(tag))
+  }


dispatchSetHeading(tag) does not validate tag before passing it to $createHeadingNode(tag). Since setHeading is now a registered command, callers can pass invalid values (or values not allowed by the configured headings), which can throw or bypass the configured restriction. Consider rejecting non-/^h[1-6]$/ tags (and optionally enforcing membership in #configuredHeadings) before applying.

This seems like we're defensively programming against the previously configured and validated heading elements. If y'all think this is valuable to implement here as well I will

lylo · 2026-02-26T09:30:43Z

If this gets merged, I'd value the ability to configure which headings are available as I want to prevent h1 being used by my customers :)

- adjust styles to give more consistent results - ensure valid headings are passed in as an array - clean up test that was formatted by prettier

ShugKnight24 · 2026-02-26T17:24:06Z

Made some updates that Copilot suggested. I think the last one is a bit redundant as we're setting an array of already configured and validated heading elements. I can adjust things as needed

I likely I ran Prettier which changed the formatting of the test - tests passing after my changes

Hey @lylo, thanks for the comment. My thought process was give access to all headings out of the box and folks could configure things as they desired. I could see the case where a h1 already exists on a page or you'd want to prevent users from adding more than 1.

Is the original h2-h4 default better? Would your customers use h5 or h6 tags? I suppose it's ultimately irrelevant as it's configurable. The default should be whatever is most useful. Happy to update this based on the preference of the community

samuelpecher · 2026-02-26T18:13:02Z

Hey @ShugKnight24 I think there's two features here:

Configuring the headings available
New UI to toggle/select heading status

The configuration feature will be simpler to land, and I think keeping the default h2-h4 is the way to go.

Given the UI changes are a more complex piece, it's worth considering proposing the configuration as a separate PR and targeting this branch into that.

- users can still configure headings as desired

ShugKnight24 · 2026-02-26T18:21:07Z

@samuelpecher, looks like we're on the same wavelength in regards to the headings

Copilot

Pull request overview

Copilot reviewed 10 out of 15 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T18:25:03Z

src/config/lexxy.js

    multiLine: true,
    richText: true,
    toolbar: true,
+    headings: [ "h2", "h3", "h4" ],


PR description says the defaults were changed to H1–H6, but the code sets the default preset headings to ["h2", "h3", "h4"]. Either update the description to match the implementation, or adjust the default headings preset here (and any other defaults) to reflect the intended behavior.

Copilot · 2026-02-26T18:25:03Z

src/editor/command_dispatcher.js

+  dispatchSetHeading(tag) {
+    const selection = $getSelection()
+    if (!$isRangeSelection(selection)) return
+
+    if (!tag) {
+      this.contents.removeFormattingFromSelectedLines()
+      return
+    }
+
+    if ($isRootOrShadowRoot(selection.anchor.getNode())) {
+      selection.insertNodes([ $createHeadingNode(tag) ])
+      return
+    }
+
+    this.contents.insertNodeWrappingEachSelectedLine(() => $createHeadingNode(tag))
+  }


dispatchSetHeading(tag) passes tag straight into $createHeadingNode(tag) without validating it. Since setHeading is now a registered command, callers can dispatch it with invalid tags (or tags not allowed by configuration), which can throw at runtime and/or bypass the configured heading restrictions. Consider validating tag against /^h[1-6]$/ and (optionally) this.#configuredHeadings before applying; otherwise treat it as a no-op or fall back to removing formatting.

No need; users should configure it correctly.

…ormat` - remove select heading UI changes

ShugKnight24 · 2026-02-26T18:51:57Z

@samuelpecher thanks for your feedback, I removed the UI changes and kept the configuration ability

If there is a decision on the desired UI let me know and I'll take a look

Cheers!

jorgemanrubia

Lovely @ShugKnight24 👏. Could you please also document the new option in the docs.

And also rebuild the assets please 🙏.

ShugKnight24 · 2026-03-03T15:17:32Z

Sure, @jorgemanrubia, thank you for the review - I'll take a look this afternoon and update the docs around the configuration

Realizing there are some conflicts in the generated files. I'll bring in the latest from main and regenerate to see if this fixes things

samuelpecher

Hey @ShugKnight24, thanks for paring this down; I like the config. When you rebase, I found some opportunitied to tighten the code up a bit.

src/editor/command_dispatcher.js

src/editor/selection.js

src/editor/command_dispatcher.js

samuelpecher · 2026-03-03T18:00:53Z

src/editor/command_dispatcher.js

+  dispatchSetHeading(tag) {
+    const selection = $getSelection()
+    if (!$isRangeSelection(selection)) return
+
+    if (!tag) {
+      this.contents.removeFormattingFromSelectedLines()
+      return
+    }
+
+    if ($isRootOrShadowRoot(selection.anchor.getNode())) {
+      selection.insertNodes([ $createHeadingNode(tag) ])
+      return
+    }
+
+    this.contents.insertNodeWrappingEachSelectedLine(() => $createHeadingNode(tag))
+  }


No need; users should configure it correctly.

test/javascript/editor/headings_configuration.test.js

samuelpecher · 2026-03-03T18:17:08Z

test/javascript/editor/headings_configuration.test.js

I ❤️ the use of presets here

🙏🏻 - Thank copilot. I originally had each in the various tests and it pointed out I could have a preset based on the codebase

samuelpecher · 2026-03-03T18:18:30Z

src/editor/command_dispatcher.js

+    const configured = this.editorElement.config.get("headings")
+    const headings = Array.isArray(configured) ? configured : [ "h2", "h3", "h4" ]
+    return headings.filter((heading) => /^h[1-6]$/.test(heading))


I feel like this is duplcating the default value. Mis-configuration producing errors is ok I think.

Yeah - this is the one aspect having automated code review kind of misses and adds junk. Has you defensively programming around ghost scenarios that will likely never happen. Thanks for your thinking here

src/editor/command_dispatcher.js

ShugKnight24 · 2026-03-03T18:32:42Z

Thanks for the thorough review @samuelpecher - will dig into your comments

- Simplify heading rotation using array index mechanics (indexOf + 1 with ?? fallback) - Use nearestNodeOfType(HeadingNode) instead of getTopLevelElementOrThrow() - Simplify headingTag in selection.js using optional chaining with $getNearestNodeOfType - Remove defensive Array.isArray guard and regex filter from #configuredHeadings - Return false from command handler early exits per Lexical convention - Remove redundant blog preset test that duplicated defaults - Rebuild assets

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

ShugKnight24 · 2026-03-03T19:08:07Z

@jorgemanrubia - added configuration to the docs in 9478bbb, let me know if you'd like that updated

@samuelpecher - addressed your review in 1ee1a03

~~I have some conflicts - let me see if I can resolve~~

Think we're all set here - let me know if either of you have any additional asks and I'm happy to address them

Cheers

Copilot

Pull request overview

Copilot reviewed 6 out of 12 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/editor/command_dispatcher.js

Copilot · 2026-03-03T19:38:35Z

src/editor/command_dispatcher.js

+  get #configuredHeadings() {
+    return this.editorElement.config.get("headings")
+  }
+
  dispatchRotateHeadingFormat() {
    const selection = $getSelection()
-    if (!$isRangeSelection(selection)) return
+    if (!$isRangeSelection(selection)) return false
+
+    const headings = this.#configuredHeadings
+    if (headings.length === 0) return false



#configuredHeadings returns the raw config value. Because EditorConfiguration falls back to returning the attribute string when JSON parsing fails, headings="h2" would yield a string here; headings.length would be non-zero and headings[0] would become 'h', causing $createHeadingNode('h') to throw. Consider normalizing in #configuredHeadings (e.g., Array.isArray(...) ? ... : []) and filtering to valid /^h[1-6]$/ strings before use.

@samuelpecher said users having errors with their own configuration is fine

src/editor/command_dispatcher.js

… rotation - Remove unused $isHeadingNode import (replaced by nearestNodeOfType) - Restore heading→paragraph cycle: clicking Heading on the last configured level now removes formatting instead of wrapping back to the first heading - Fixes the existing "rotate headers" system test expectation (h4 → p)

feat: enable headings to be configurable

57310e4

- Closes basecamp#614

Copilot AI review requested due to automatic review settings February 26, 2026 07:44

Copilot started reviewing on behalf of ShugKnight24 February 26, 2026 07:44 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

fix: address copilot

9c4db30

- top level configure - test h1 - test empty case - validate headings to prevent other elements from being used - revert previous whitespace changes

ShugKnight24 requested a review from Copilot February 26, 2026 08:15

Copilot started reviewing on behalf of ShugKnight24 February 26, 2026 08:15 View session

ShugKnight24 commented Feb 26, 2026

View reviewed changes

style: remove whitespace formatting

1c1349d

Copilot AI reviewed Feb 26, 2026

View reviewed changes

fix: address copilot review

b224f00

- adjust styles to give more consistent results - ensure valid headings are passed in as an array - clean up test that was formatted by prettier

refactor: revert to original h2-h4 heading defaults

ca39bff

- users can still configure headings as desired

Copilot AI review requested due to automatic review settings February 26, 2026 18:19

Copilot started reviewing on behalf of ShugKnight24 February 26, 2026 18:20 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

refactor: revert to origin heading functionality with `rotateHeadingF…

5c60067

…ormat` - remove select heading UI changes

jorgemanrubia approved these changes Mar 3, 2026

View reviewed changes

ShugKnight24 added 2 commits March 3, 2026 13:11

Merge main into feat/configurable_headings and rebuild assets

b370be1

docs: add headings option to configuration docs

9478bbb

samuelpecher reviewed Mar 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 3, 2026 19:02

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Merge origin/main into feat/configurable_headings and rebuild assets

ed93022

ShugKnight24 requested a review from Copilot March 3, 2026 19:32

Copilot started reviewing on behalf of ShugKnight24 March 3, 2026 19:33 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

ShugKnight24 mentioned this pull request Mar 3, 2026

Feat/headings dropdown UI #776

Draft

Conversation

ShugKnight24 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lylo commented Feb 26, 2026

Uh oh!

ShugKnight24 commented Feb 26, 2026

Uh oh!

samuelpecher commented Feb 26, 2026

Uh oh!

ShugKnight24 commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShugKnight24 commented Feb 26, 2026

Uh oh!

jorgemanrubia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShugKnight24 commented Mar 3, 2026

Uh oh!

samuelpecher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

ShugKnight24 commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

ShugKnight24 commented Feb 26, 2026 •

edited

Loading

jorgemanrubia left a comment •

edited

Loading

ShugKnight24 commented Mar 3, 2026 •

edited

Loading