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

fix: respect the top-level copyButtons option #990

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

rafegoldberg
Copy link
Contributor

@rafegoldberg rafegoldberg commented Oct 9, 2024

🎫 Resolves RM-10635 🐙 Closes #964

🧰 Changes

The main RMDX.compile() method should respect the copyButtons param when rendering our code block components.

🧬 QA & Testing

Pull this PR down, run the start script, and navigate to the “Code Blocks” example. Try hovering over a code block to verify that the copy button is only shown if/when the Copy Buttons toggle is checked.

Footnotes

  1. @kellyjosephprice—do we even respect those other two global config options? I think safeMode and lazyImages both have toggles in the playground, but there are a bunch of others which I'm not sure are still valid either? Anyways, just an aside, but it would be nice to clean this all up at some point!

rafegoldberg and others added 3 commits October 9, 2024 05:58
the RMDX compiler will now accept a `copyButtons` boolean within the options arg and properly pass it down to our custom `<Code>` component
pull over some polish work from #964

Co-authored-by: Trisha Le <[email protected]>
@rafegoldberg rafegoldberg added the bug Something isn't working label Oct 9, 2024
@kellyjosephprice
Copy link
Collaborator

  1. @kellyjosephprice—do we even respect those other two global config options? I think safeMode and lazyImages both have toggles in the playground, but there are a bunch of others which I'm not sure are still valid either? Anyways, just an aside, but it would be nice to clean this all up at some point!

I think the still valid options are:

  copyButtons: true,
  markdownOptions: {
    fences: true,
    commonmark: true,
    gfm: true,
    ruleSpaces: false,
    listItemIndent: '1',
    spacedTable: true,
    paddedTable: true,
  },
  lazyImages: true,,
  theme: 'light',

I don't think we're passing markdownOptions in correctly, or if they're they're up to date with the latest options.

Copy link
Collaborator

@kellyjosephprice kellyjosephprice left a comment

Choose a reason for hiding this comment

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

LGTM.

Just now seeing that Settings is not considered extensible.

Another alternative would be to pass this down through a context. This is great though.

Copy link
Member

@trishaprile trishaprile left a comment

Choose a reason for hiding this comment

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

🙏

@rafegoldberg rafegoldberg merged commit 1242413 into next Oct 11, 2024
13 checks passed
@rafegoldberg rafegoldberg deleted the fix/respect-the-copyButtons-option branch October 11, 2024 15:45
@rafegoldberg
Copy link
Contributor Author

(gonna hold of on cutting a new release for this pending #992 so we can hopefully kill two birds w/one stone)

rafegoldberg pushed a commit that referenced this pull request Oct 14, 2024
## Version 7.6.7

### ✨ New & Improved

* **Image:** match new alignment selectors ([#992](#992)) ([73bca2b](73bca2b))

### 🛠 Fixes & Updates

* better tag handling ([#991](#991)) ([e6aa82d](e6aa82d))
* respect the top-level `copyButtons` option ([#990](#990)) ([1242413](1242413)), closes [#964](#964) [/github.com/readmeio/markdown/blob/96f9644f04e6d8e3ffff6f9c014432f901c0b804/lib/compile.ts#L28](https://github.com/readmeio//github.com/readmeio/markdown/blob/96f9644f04e6d8e3ffff6f9c014432f901c0b804/lib/compile.ts/issues/L28) [/github.com/readmeio/markdown/blob/96f9644f04e6d8e3ffff6f9c014432f901c0b804/lib/compile.ts#L28](https://github.com/readmeio//github.com/readmeio/markdown/blob/96f9644f04e6d8e3ffff6f9c014432f901c0b804/lib/compile.ts/issues/L28) [/github.com/readmeio/markdown/blob/b9502adb306f099cd91e005df17c0be252019814/options.js#L1-L22](https://github.com/readmeio//github.com/readmeio/markdown/blob/b9502adb306f099cd91e005df17c0be252019814/options.js/issues/L1-L22)

### 📘 Tests & Docs

* readme updates ([#993](#993)) ([3a4e30d](3a4e30d))

<!--SKIP CI-->
@rafegoldberg
Copy link
Contributor Author

This PR was released!

🚀 Changes included in v7.6.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants