ethereum / ethereum-org-fork

Ethereum.org is a primary online resource for the Ethereum community.
https://ethereum-org-fork.netlify.app/
MIT License
10 stars 3 forks source link

Bug report #9

Closed nhsz closed 1 year ago

nhsz commented 1 year ago

Describe the bug

We have some issues related to the styles of DocLink and ButtonLink components

To reproduce

Expected behavior

Proper styling

Screenshots

DocLink

Screen Shot 2023-08-16 at 15 49 55

ButtonLink

Screen Shot 2023-08-16 at 15 50 09

Desktop (please complete the following information)

No response

Smartphone (please complete the following information)

No response

Additional context

No response

Would you like to work on this issue?

pettinarip commented 1 year ago

This problem is related to the new version of mdx that we are using here, v2.

The current syntax that we are using works with mdx v1. Since we have upgraded it to v2, a few things have changed. For example, content that is inside jsx is now considered markdown, so:

<Button>
  See our other guides
</Button>

// in v1, this is converted to:
<button>
  See our other guides
</button>

// in v2, the new line will means that this is a new paragraph:
<button>
  <p>See our other guides</p>
</button>

Then what we should do to keep the previous form in mdx v2, would be:

// avoid the new line
<Button>See our other guides</Button>

// or we could use this form to avoid being parsed as markdown
// https://mdxjs.com/migrating/v2/#expressions
{
<Button>
  See our other guides
</Button>
}

More about changes in v2: https://mdxjs.com/migrating/v2/#update-mdx-files


IMO given that fixing this issue would imply changes in a lot of md pages (and a lot of sync conflicts with upstream), I'd propose not doing this mdx migration now and creating its own epic/task to be tackled after we finish the current NextJS migration.

This would mean to downgrade mdx to v1, which means that we'd need to downgrade next-mdx-remote to v3.

cc @nhsz @corwintines @wackerow @minimalsm

pettinarip commented 1 year ago

If we all agree then I will reopen https://github.com/ethereum/ethereum-org-website/issues/9160 to keep track of that.

nhsz commented 1 year ago

IMO given that fixing this issue would imply changes in a lot of md pages (and a lot of sync conflicts with upstream), I'd propose not doing this mdx migration now and creating its own epic/task to be tackled after we finish the current NextJS migration.

@pettinarip thanks for the research. Agree to downgrade next-mdx-remote to v3 for now and migrate to mdx v2 later. We would need to test that next-mdx-remote v3 doesnt break the build or introduce other bugs

pettinarip commented 1 year ago

@nhsz cool, I can tackle that. Will create a new card.

On the other hand, I'll reopen the mentioned issue.

nhsz commented 1 year ago

@pettinarip re-opening as there's still a minor difference with the styling of DocLink component, maybe related to the updated Link component

now rendering

Screen Shot 2023-09-05 at 10 49 34

should be

Screen Shot 2023-09-05 at 10 51 59
nhsz commented 1 year ago

Fixed now by #21, thx!