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: add dynamic theme colors and attr_list support for paging #21

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

yves-chevallier
Copy link

I was about to write another Draw.io plugin for MkDocs when I came across yours. It's simple and effective, but I found myself missing a few features:

  • Avoid using the alt text of the image for pages, as I'm using mkdocs-caption.
  • Adjust the drawing theme automatically based on the color scheme (light/dark).
  • Remove specifics tied to the theme in use (e.g., mkdocs/material).

To maintain backward compatibility, I've added an alt_as_page option, which defaults to true. When set to false, the page can instead be specified using an attribute (requires attr_list to be enabled):

![Image Caption](image.png){ page="pageName" }

I also took the opportunity to modernize the configuration by using the DrawioConfig(base.Config) class.

Lastly, your plugin currently works only with light themes. Fortunately, Draw.io recently added support for the color-scheme attribute in their viewer, allowing for theme adjustments. Since this requires a bit of JavaScript and CSS, the plugin now automatically copies the necessary files, so there’s no need to add any extra resources manually.

This is just a draft proposal for now—let me know your thoughts!

Copy link
Owner

@tuunit tuunit left a comment

Choose a reason for hiding this comment

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

Overall great addition to the project 👏🏼
Just some nit-picks and questions.

@@ -1,32 +1,41 @@
<mxfile host="app.diagrams.net" agent="Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36" version="24.7.7">
<mxfile host="Electron" agent="Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) draw.io/24.7.8 Chrome/128.0.6613.36 Electron/32.0.1 Safari/537.36" version="24.7.8">
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<mxfile host="Electron" agent="Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) draw.io/24.7.8 Chrome/128.0.6613.36 Electron/32.0.1 Safari/537.36" version="24.7.8">
<mxfile>

Copy link
Author

Choose a reason for hiding this comment

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

I understand from what it comes. I am using the standalone version Draw.io which is using Electron instead of the browser. Yes this should be removed, but the page and version attribute should remain in the tag <mxfile>. We can fix all drawings with a script:

import os
import xml.etree.ElementTree as ET

def clean_drawio_files(directory="."):
    """
    Look for all .drawio files in the given directory and its subdirectories and
    remove the host and agent attributes from the mxfile element if they exist.
    """
    for root, _, files in os.walk(directory):
        for file in files:
            if file.endswith(".drawio"):
                file_path = os.path.join(root, file)
                try:
                    tree = ET.parse(file_path)
                    root_element = tree.getroot()

                    if root_element.tag == "mxfile":
                        changed = False
                        for attr in ["host", "agent"]:
                            if attr in root_element.attrib:
                                del root_element.attrib[attr]
                                changed = True

                        if changed:
                            tree.write(file_path, encoding="utf-8", xml_declaration=True)
                            print(f"Nettoyé : {file_path}")
                        else:
                            print(f"Aucun changement : {file_path}")
                except Exception as e:
                    print(f"Erreur lors du traitement de {file_path} : {e}")

if __name__ == "__main__":
    clean_drawio_files()

Copy link
Author

Choose a reason for hiding this comment

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

I have added that script it can be run with:

poetry run python scripts/clean_drawio_files.py

@@ -68,7 +79,8 @@ plugins:
toolbar: true # control if hovering on a diagram shows a toolbar for zooming or not (default: true)
tooltips: true # control if tooltips will be shown (default: true)
edit: true # control if edit button will be shown in the lightbox view (default: true)
border: 10 # increase or decrease the border / margin around your diagrams (default: 5)
border: 10 # increase or decrease the border / margin around your diagrams (default: 5)
alt_as_page: true # use the alt text as page name (default: false)
Copy link
Owner

Choose a reason for hiding this comment

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

Small thing but can you indent all comments to the same level?

Copy link
Author

Choose a reason for hiding this comment

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

Sure

@tuunit tuunit changed the title Add config option, light refactoring, dynamic theme colors. feat: add dynamic theme colors and attr_list support for paging Jan 23, 2025
yves-chevallier and others added 3 commits January 23, 2025 12:13
Co-authored-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
Comment on lines +8 to +13
if (typeof document$ !== "undefined" && typeof document$.subscribe === "function") {
document$.subscribe(({ body }) => {
GraphViewer.processElements()
colorize();
});
} else {
Copy link
Owner

Choose a reason for hiding this comment

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

This will automatically back in support for the instant reload of material themes. Therefore the explanation for how to manually add it should be removed from the README.

Furthermore, I would like to have a comment explaining the necessity of this if-else block. Because a couple of weeks or months down the round no one will remember why this code exists.

@yves-chevallier
Copy link
Author

I have made significant changes. Let me know what you think!

I added features that allow customizing the appearance of the drawing. The test site was renamed to “documentation.” In principle (though not tested yet), the documentation is automatically uploaded to GitHub Pages. Since it is now a complete plugin documentation, it can be made available to all users.

I’m going on vacation for three weeks, so let’s discuss everything when I get back. If you agree with these changes, you could bump the version to 2.0.0, given how extensive they are compared to the original plugin.

For your information, I am interested to this plugin for my online course (https://heig-tin-info.github.io/handbook/) which uses a lot of draw.io. I am currently using a home made hook and the idea is to rely on a standalone plugin for drawio.

@tuunit
Copy link
Owner

tuunit commented Jan 25, 2025

I have made significant changes. Let me know what you think!

I added features that allow customizing the appearance of the drawing. The test site was renamed to “documentation.” In principle (though not tested yet), the documentation is automatically uploaded to GitHub Pages. Since it is now a complete plugin documentation, it can be made available to all users.

I’m going on vacation for three weeks, so let’s discuss everything when I get back. If you agree with these changes, you could bump the version to 2.0.0, given how extensive they are compared to the original plugin.

For your information, I am interested to this plugin for my online course (https://heig-tin-info.github.io/handbook/) which uses a lot of draw.io. I am currently using a home made hook and the idea is to rely on a standalone plugin for drawio.

Hi @yves-chevallier,

thanks again for contributing and the changes you made are amazing ⭐ but we should have introduced the changes in multiple PRs. This way people could gradually adopt the changes and published versions. Furthermore from a review perspective this PR got out of hand.

Don't get me wrong. The work and cleanup you have done looks great and we will make it work.

@yves-chevallier
Copy link
Author

Yes I agree, once I come back I close the PR and reopen some with gradual changes

Copy link
Owner

@tuunit tuunit left a comment

Choose a reason for hiding this comment

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

Haven't reviewed everything yet.

Comment on lines +18 to +19
- name: Install Poetry
uses: snok/install-poetry@v1
Copy link
Owner

Choose a reason for hiding this comment

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

Why unnecessary introduce another dependency to such a small project?
We can just do the install of poetry as before.

      - name: Install pypa/build
        run: python3 -m pip install build poetry --user

Comment on lines +42 to +47
- name: Build and Deploy
uses: JamesIves/github-pages-deploy-action@4.7.2
with:
ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }}
BRANCH: gh-pages
FOLDER: documentation/site
Copy link
Owner

Choose a reason for hiding this comment

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

Same goes for this one. Lets use the official GitHub Action for deploying pages:
https://github.com/actions/deploy-pages


## Usage

Simply add an image as you would normally do in markdown:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Simply add an image as you would normally do in markdown:
Simply add your drawio files like you would any other image in markdown:

Comment on lines +5 to +7
## Diagrams.net

**Diagrams.net** previoully known as **Draw.io** is a free online diagram software. It is perfect for creating diagrams, flowcharts, process diagrams, and more. It is a powerful tool that can be used for creating diagrams for any purpose.
Copy link
Owner

Choose a reason for hiding this comment

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

Never blindly trust LLMs ;)

Suggested change
## Diagrams.net
**Diagrams.net** previoully known as **Draw.io** is a free online diagram software. It is perfect for creating diagrams, flowcharts, process diagrams, and more. It is a powerful tool that can be used for creating diagrams for any purpose.
## Draw.io
**Draw.io** previoully known as **Diagrams.net** is a free online diagram software. It is perfect for creating diagrams, flowcharts, process diagrams, and more.


**Diagrams.net** previoully known as **Draw.io** is a free online diagram software. It is perfect for creating diagrams, flowcharts, process diagrams, and more. It is a powerful tool that can be used for creating diagrams for any purpose.

It relies on JTGraph's [mxGraph](https://jgraph.github.io/mxgraph/) library to render the diagrams. A mxGraph is a XML-based language that defines the structure of the diagram. Here an example
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
It relies on JTGraph's [mxGraph](https://jgraph.github.io/mxgraph/) library to render the diagrams. A mxGraph is a XML-based language that defines the structure of the diagram. Here an example
It relies on JGraph's [mxGraph](https://jgraph.github.io/mxgraph/) library to render the diagrams. A mxGraph uses XML to define the structure of diagrams. Here an example


## GraphViewer

The plugin uses the [GraphViewer](https://github.com/jgraph/drawio/blob/dev/src/main/webapp/js/viewer-static.min.js) minified version of the drawio viewer.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
The plugin uses the [GraphViewer](https://github.com/jgraph/drawio/blob/dev/src/main/webapp/js/viewer-static.min.js) minified version of the drawio viewer.
The plugin uses the minified version of the [drawio viewer](https://github.com/jgraph/drawio/blob/dev/src/main/webapp/js/viewer-static.min.js).

@@ -0,0 +1,7 @@
# Code Blocks

Diagrams are not rendered in code blocks. So you don't have to worry about the diagram being rendered in the code block.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Diagrams are not rendered in code blocks. So you don't have to worry about the diagram being rendered in the code block.
Diagrams are not rendered in code blocks. So you don't have to worry if you want to explain how to embed drawio diagrams in your own docs.

Comment on lines +57 to +60
border = c.Optional(c.Type(int))
padding = c.Type(int, default=10)
""" Padding around the diagram, border will be deprecated
but kept for backwards compatibility """
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
border = c.Optional(c.Type(int))
padding = c.Type(int, default=10)
""" Padding around the diagram, border will be deprecated
but kept for backwards compatibility """
border = c.Optional(c.Type(int))
""" Deprecated: Use the padding option instead """
padding = c.Type(int, default=10)
""" Padding around the diagram """

Copy link
Owner

Choose a reason for hiding this comment

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

Please remove the poetry.lock file and put it into the .gitignore

Comment on lines +34 to +35
poetry = { version = "^2.0", python = ">=3.9" } # Marqueur d'environnement
black = { version = ">=24.0", python = ">=3.9" } # Pareil pour black
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
poetry = { version = "^2.0", python = ">=3.9" } # Marqueur d'environnement
black = { version = ">=24.0", python = ">=3.9" } # Pareil pour black
poetry = { version = "^2.0", python = ">=3.9" }
black = { version = ">=24.0", python = ">=3.9" }

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.

2 participants