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

Add possibility to ignore based on existing class #213

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

aepfli
Copy link
Contributor

@aepfli aepfli commented Mar 17, 2023

Ingoring by tag attribute is helpful, but eg. docusaurus does not offer a good way to manipulate its theme. But it offers a way to always attach a custom class.

With this approach, we will reuse the IgnoreTagAttribute as a class name detection.

It isn't worth specifying an additional attribute for this use case, although the naming might be off now.

Disclaimer
I did not find a contributing guide, so I went ahead and created this pr. If you prefer to discuss this first in an issue, I will also gladly create one.

Ingoring by tag attribute is really helpful, but eg. docusaurus
does not offer a good way to manipulate its theme. But it offers
a way to always attach a custom class.

With this approach we will reuse the IgnoreTagAttribute also as
a detection for classname.

I do not think it is worth to specify an additional attribute for
this use-case althought the naming might be off now.

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Copy link
Owner

@wjdp wjdp left a comment

Choose a reason for hiding this comment

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

Change looks good, happy to merge.

Re. issue vs PR: I should probably write a contributing guide. For something like this it kinda depends on whether you're happy to do the code upfront. It makes it super easy to give a go/no-go from my side but obvs only works if contributor is happy to spend that time and diff is small.

@wjdp wjdp merged commit 19a1f6f into wjdp:master Mar 17, 2023
@aepfli
Copy link
Contributor Author

aepfli commented Mar 17, 2023

thank you! - if there should be any regression or follow-up just let me know, i am always glad to help

@wjdp
Copy link
Owner

wjdp commented Mar 17, 2023

Will do!

@aepfli
Copy link
Contributor Author

aepfli commented Mar 21, 2023

Disclaimer: I am not in a hurry :) I am just curious :)

Do you know when this might be released?

@wjdp
Copy link
Owner

wjdp commented Mar 22, 2023

Soon but uncertain, when I get around to sitting down and doing it. I tend to check on issues/PRs here when I'm not in a position to actually do it so it tends to get forgotten 😞 Will try and do within week or so.

In the meantime if you can use the linux or mac versions CI builds you can grab from https://github.com/wjdp/htmltest/actions/runs/4448775063

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