Skip to content

Conversation

KristofferStrube
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

PR Title
Fixes that Blazor focus extension could not focus SVG elements

PR Description
I have added an extra case in the typescript wrapper for the focus extension that checks if the parsed element is an SVG element and also if it has a tabindex.
I have added a simple test that covers this behaviour.

Fixes #35046

@KristofferStrube KristofferStrube requested a review from a team as a code owner August 5, 2021 21:57
@ghost ghost added area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member labels Aug 5, 2021
@dnfadmin
Copy link

dnfadmin commented Aug 5, 2021

CLA assistant check
All CLA requirements met.

Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Looks great! 😃 Just a small wording suggestion.

@pranavkm pranavkm enabled auto-merge (squash) August 5, 2021 22:52
@KristofferStrube
Copy link
Contributor Author

Many thanks for the help @MackinnonBuck
I'm new to this so also had to learn when to pull upstream and when not to.

@KristofferStrube
Copy link
Contributor Author

KristofferStrube commented Aug 5, 2021

I was also a bit unsure if I had to commit the files in
https://github.com/KristofferStrube/aspnetcore/tree/main/src/Components/Web.JS/dist/Release
that are generated when building so I did not.

@MackinnonBuck
Copy link
Member

@KristofferStrube You've done a great job identifying the necessary changes 🙂

I was also a bit unsure if I had to commit the files in
https://github.com/KristofferStrube/aspnetcore/tree/main/src/Components/Web.JS/dist/Release
that are generated when building so I did not.

Yes, we usually do commit those files. CI might fail if you don't. Would you be able to do so?

@KristofferStrube
Copy link
Contributor Author

Yes, we usually do commit those files. CI might fail if you don't. Would you be able to do so?

Yep, I will get right on it but will have to build again locally first.

auto-merge was automatically disabled August 5, 2021 23:46

Head branch was pushed to by a user without write access

@MackinnonBuck MackinnonBuck enabled auto-merge (squash) August 5, 2021 23:46
@KristofferStrube
Copy link
Contributor Author

Hey @MackinnonBuck some of the tests failed due to what I think was a timeout or unlucky disc access errors. Can the CI tasks that failed be retriggered somehow or should I do something to fix this?

@MackinnonBuck MackinnonBuck merged commit e932a8a into dotnet:main Aug 6, 2021
@ghost ghost added this to the 6.0-rc1 milestone Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: Unable to focus on an invalid element when focusing on SVG elements

3 participants