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

How to disable internal Attributes for component.MarkupMatches(expected); #111

Closed
samtrion opened this issue Apr 29, 2020 · 7 comments · Fixed by #112
Closed

How to disable internal Attributes for component.MarkupMatches(expected); #111

samtrion opened this issue Apr 29, 2020 · 7 comments · Fixed by #112
Labels
bug Something isn't working enhancement New feature or request question Further information is requested

Comments

@samtrion
Copy link
Contributor

samtrion commented Apr 29, 2020

Is there a way to exclude the Blazor internal attributes __internal_stopPropagation_<eventname> and __internal_preventDefault_<eventname> from the component.MarkupMatches(expected); without marking each occurrence with :ignore?

@samtrion samtrion added the question Further information is requested label Apr 29, 2020
@samtrion
Copy link
Contributor Author

Maybe with a code extension here? Something like this

public static bool IsBlazorAttribute(string attributeName)
    => attributeName.StartsWith(BLAZOR_ATTR_PREFIX, StringComparison.Ordinal)
    || attributeName.StartsWith("__internal_stopPropagation_", StringComparison.Ordinal)
    || attributeName.StartsWith("__internal_preventDefault_", StringComparison.Ordinal);

@egil egil added bug Something isn't working enhancement New feature or request labels Apr 29, 2020
@egil
Copy link
Member

egil commented Apr 29, 2020

Hey @samtrion

Yeah, the specific attributes Blazor adds to the markup is handled in the Htmlizer class, so something like that would need to be added there.

Can you show me the component that generates the markup?

I will definitely add a fix in the next release.

ps. thanks for bringing this to my attention!

@egil egil added this to the beta-7 milestone Apr 29, 2020
@egil
Copy link
Member

egil commented Apr 29, 2020

And to answer your question, yes, I think the best solution for now is to use :ignore everywhere for now. I do hope to have a new release in a few weeks, that could include this, so they at that point become redundant.

If you want to help out with a PR on this, I would appreciate it. PRs should target the DEV branch.

Without having given much thought to a solution, I think that it should actually be handled in the same way as event-handler-trackers, by changing the attribute name to have the BLAZOR_ATTR_PREFIX prefix, making it play nice with the existing comparison/diffing logic, without any additions, probably adding another specific case to the switch statement in the file: https://github.com/egil/bunit/blob/master/src/Rendering/Internal/Htmlizer.cs#L220

@samtrion
Copy link
Contributor Author

Will create a PR.

Use an internally created variant for now, until the next release.

@egil
Copy link
Member

egil commented Apr 30, 2020

Awesome, much appreciated. Be warned, the project structure in the DEV branch has changed significantly. You will find Htmlizer.cs in the bunit.web project now - https://github.com/egil/bunit/blob/dev/src/bunit.web/Rendering/Internal/Htmlizer.cs.

@samtrion
Copy link
Contributor Author

PR was created.

Sorry about the Spacing Disaster.

@egil
Copy link
Member

egil commented Apr 30, 2020

NO worries. Added my review. Ill just squash in the end.

@egil egil linked a pull request Apr 30, 2020 that will close this issue
egil pushed a commit that referenced this issue Apr 30, 2020
* Fixed Internal Attributes #111

Signed-off-by: samtrion <[email protected]>

* Fixed spacing

* Fixed spacing, second try

* I'm so sorry, please squash

* Update src/bunit.web/Rendering/Internal/Htmlizer.cs

Co-Authored-By: Egil Hansen <[email protected]>

* Added theory for handling internal attributes

Signed-off-by: samtrion <[email protected]>

* Added explanation

Signed-off-by: samtrion <[email protected]>

* Added test, which confirms correctly prefixed attributes

Signed-off-by: samtrion <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants