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

[feat!] Get lineProps, lineNumberProps, and lineNumberContainerProps back #320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmvilas
Copy link

@fmvilas fmvilas commented Oct 14, 2020

  • Initial peek
  • Fix tests and snapshots.
  • Add lineNumberContainerProps.
  • Update demos.

Fixes #311

@fmvilas
Copy link
Author

fmvilas commented Oct 14, 2020

Hey @simmerer. I got a rough version of what it would look like to bring back the missed props. Would need some help or guidance on how to fix the snapshots. Do you regenerate them from scratch? Also, have a look at the code and let me know if you find something weird or incorrect. Thanks!

@@ -20,6 +20,7 @@ exports[`SyntaxHighlighter component passes along code style to non-inline line
}
}
>
<<<<<<< HEAD
Copy link
Author

Choose a reason for hiding this comment

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

To be done.

@@ -3304,66 +3304,106 @@ exports[`SyntaxHighlighter component renders line numbers if showLineNumbers ===
1
</span>
<span
<<<<<<< HEAD
Copy link
Author

Choose a reason for hiding this comment

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

To be done.

@simmerer
Copy link
Collaborator

@fmvilas If I'm confident that rendering changes are correct and the snapshots need to be updated, then I run jest -u locally to update the snapshots.

I'll take a look soon. Thanks for starting on this!

@SalahAdDin
Copy link

I'm getting this bug right now:

react-dom.development.js?61bb:88 Warning: React does not recognize the `lineNumberContainerProps` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `linenumbercontainerprops` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
    in pre (created by SyntaxHighlighter)
    in SyntaxHighlighter (at content.js:58)
    in CodeBlock (created by ReactMarkdown)
    in Root (created by ReactMarkdown)
    in ReactMarkdown (at content.js:163)
    in Content (at body.js:26)
    in section (created by ForwardRef(Typography))
    in ForwardRef(Typography) (created by WithStyles(ForwardRef(Typography)))
    in WithStyles(ForwardRef(Typography)) (at body.js:18)
    in DynamicZone (at body.js:55)
    in Body (at [slug].js:98)
    in main (created by ForwardRef(Container))
    in ForwardRef(Container) (created by WithStyles(ForwardRef(Container)))
    in WithStyles(ForwardRef(Container)) (at layout.js:10)
    in Layout (at [slug].js:40)
    in Post (at _app.js:55)
    in ThemeProvider (at _app.js:52)
    in FolioApp (created by withI18nextSSR(FolioApp))
    in withI18nextSSR(FolioApp) (created by AppWithTranslation)
    in NextStaticProvider (created by withI18nextTranslation(NextStaticProvider))
    in withI18nextTranslation(NextStaticProvider) (created by AppWithTranslation)
    in I18nextProvider (created by AppWithTranslation)
    in AppWithTranslation (created by withRouter(AppWithTranslation))
    in withRouter(AppWithTranslation)
    in ErrorBoundary (created by ReactDevOverlay)
    in ReactDevOverlay (created by Container)
    in Container (created by AppContainer)
    in AppContainer
    in Root

Does this pull request to solve it?

@simmerer
Copy link
Collaborator

@SalahAdDin I see that this PR has a few merge conflicts, and now that we've introduced the showInlineLineNumbers prop, I'll need to make sure this older API for non-inline line numbers plays nice with the new one. I'll work on it this week.

@SalahAdDin
Copy link

@simmerer Any progress about this? Thanks.

@SalahAdDin
Copy link

@fmvilas what happened whit thi one?

@fmvilas
Copy link
Author

fmvilas commented Feb 5, 2024

Sorry got completely neck deep into AsyncAPI work. Don't have time to contribute to this one so feel free to take over it or close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not work with TailwindCSS anymore
4 participants