Skip to content

Conversation

@MChandrahas
Copy link

Description

This PR addresses issue #31 by replacing the images in docs/explanation/mode-of-operation.md with text descriptions.

Changes

  • Removed the introductory paragraph ("The purpose of Chisel is...") as requested in the issue.
  • Removed the HTML table and images.
  • Converted the visual workflow steps into a numbered list with descriptive text.

QA Steps

  1. Open docs/explanation/mode-of-operation.md.
  2. Verify that the introductory paragraph is removed.
  3. Verify that the images are replaced by the 4-step text workflow.

@asanvaq asanvaq self-requested a review December 10, 2025 16:24
Copy link
Contributor

@asanvaq asanvaq left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! We have been discussing this change among the team, and we reached the conclusion that a more visual alternative to the images is needed (eg. a diagram). We want to replicate the original version without using any images. I understand this adds a layer of complexity to this issue, so please let me know if you would like to continue working on this!

@MChandrahas
Copy link
Author

I would love to work on this, but before making any changes, could we discuss it in the Chisel Matrix room? I want to run my ideas through you first and proceed only if everything looks good.

@asanvaq
Copy link
Contributor

asanvaq commented Dec 12, 2025

Great! Feel free to message the Matrix room.

@MChandrahas
Copy link
Author

Thank you

@MChandrahas
Copy link
Author

I've updated PR #40 with the Mermaid diagrams. I used a vertical layout so it renders correctly on GitHub and mobile.

I initially tried a side-by-side table layout, but I found that GitHub's markdown renderer caused issues with the diagram sizing and toolbars.

Ready for your review when you have a moment!

@asanvaq
Copy link
Contributor

asanvaq commented Dec 17, 2025

Thanks for the update! The Read the docs project is not building, I suspect it's because the “sphinxcontrib-mermaid” extension is not added to requirements.txt and config.py. Instructions on how to do this can be found here.

@MChandrahas
Copy link
Author

Fixed! I added the missing "sphinxcontrib-mermaid" dependency and configured myst_fence_as_directive in conf.py. The local build is passing now without warnings

@MChandrahas
Copy link
Author

I have removed the old image files since they are no longer used. Ready for review!

Thanks for your patience and supporting me.

@asanvaq
Copy link
Contributor

asanvaq commented Dec 19, 2025

It looks great! We have been discussing this solution and there is now some repetition in the text, and we would like to avoid that. Could you try to recreate the table, but with a mermaid diagram in each cell for the title? Basically like the original but with mermaid diagrams replacing the images. Thank you for your patience while we figure this out together!

@MChandrahas
Copy link
Author

Hi @asanvaq , thanks for the feedback!

I completely agree about avoiding the text repetition. I tried to recreate the table layout as you suggested (putting the Mermaid diagrams inside the table cells), but the documentation theme is struggling to render it correctly.

As you can see in the screenshot below, the layout engine shrinks the diagrams down until they are unreadable and creates large empty gaps in the page alignment.

Here is a screenshot from my testing branch to show you what I mean:

image

Given that the table layout causes these rendering bugs, do you have any suggestions on how you'd like to proceed?

@asanvaq
Copy link
Contributor

asanvaq commented Jan 5, 2026

Hi! Apologies for the delay, we were off due to end of the year break. Thanks for the change, is this all in one table? If so, have you tried checking how it looks with a table with two columns for each step?

@MChandrahas
Copy link
Author

Thanks for the suggestion! Yes, the previous attempt was a single table. I will try splitting it into a table with two columns for each step to see if that fixes the rendering issues. I'll update the PR once I've tested it.

@MChandrahas
Copy link
Author

Fix: Mermaid diagram height issue

Problem

The mermaid extension generates inline <style> with a fixed height of 500px, causing large gaps above and below diagrams.

Solution

Added custom.js with a MutationObserver that watches for DOM changes and sets svg.style.height = 'auto' dynamically.

@asanvaq
Copy link
Contributor

asanvaq commented Jan 9, 2026

Thanks! There is still quite a big gap between the diagrams, ideally the entire text would fit on the screen, without having to scroll.

@MChandrahas
Copy link
Author

@asanvaq Thanks for the feedback!

I actually added a custom JS script in the last commit specifically to resize the diagrams dynamically so they fit the screen without scrolling. It works perfectly in my local build.

Since you are still seeing the gaps, is it possible that the custom JS isn't loading or executing in the Read the Docs deployment? Please let me know if there are specific restrictions for custom scripts here.

@rkratky
Copy link
Contributor

rkratky commented Jan 15, 2026

@MChandrahas, thanks so much for trying to resolve this. I'm afraid the complexity of the solution (custom JavaScript, Mermaid diagrams) for such a trivial thing (a fancy arrow) is getting a bit out of hand.

How about using CSS only? The following could be in place of the SVG images in the table:

<style>
.arrow-box {
  background-color: #A81C1C;
  color: white;
  padding: 15px 40px;
  clip-path: polygon(0% 0%, 92% 0%, 100% 50%, 92% 100%, 0% 100%);
}
</style>

<div class="arrow-box">
    1. Read and parse chisel-releases
</div>

Of course, the CSS definition would go into custom.css. It looks like this:
image
The angles can be fiddled with.

@MChandrahas
Copy link
Author

@rkratky Thanks for the suggestion! That does look much simpler than the JS workaround.

@asanvaq Before I apply this change, do you agree with switching to this CSS-only approach instead of the Mermaid diagrams? If so, I can close out the JS/Mermaid implementation and use this style instead.

@MChandrahas MChandrahas requested a review from asanvaq January 19, 2026 20:54
@asanvaq
Copy link
Contributor

asanvaq commented Jan 19, 2026

Yes! It sounds good :)

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.

3 participants