8000
Skip to content
This repository was archived by the owner on Dec 3, 2024. It is now read-only.

Click event handler to control resetting the component - #195

Merged
chrissainty merged 2 commits into
Blazored:mainfrom
HAL-NINE-THOUSAND:main
Jan 7, 2021
Merged

Click event handler to control resetting the component#195
chrissainty merged 2 commits into
Blazored:mainfrom
HAL-NINE-THOUSAND:main

Conversation

@HAL-NINE-THOUSAND
@HAL-NINE-THOUSAND HAL-NINE-THOUSAND commented Jan 4, 2021
Copy link
Copy Markdown
Contributor

Hi Chris,

As requested, a PR for #183

Resolves #183

@chrissainty chrissainty left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks to be working really well! Nice work @HAL-NINE-THOUSAND 😃. I've asked one little nit pick, see what you think.

When I was testing it I found the Typeahead to but a little sluggish when closing, I thought one of the Task.Delays might be causing the issue and I was right. I removed it and everything worked as expected and the speed was much much better. The offending line is 289 inside the ResetControl method. With that delay removed the method can be changed to a void type as well.

I would have added this as a code suggestion on the review but I can't make suggestions on code you didn't edit, as far as I can see. Would you be able to make this change as well? Then we can get this merged.

var bId = "";//get the blazor internal ID to distinguish different components
for (var clearCount = 0; clearCount < searchTextElement.attributes.length; clearCount++) {
var a = searchTextElement.attributes[clearCount];
if (a.name[0] == '_') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit pick: Should this be _bl_ as that is the full prefix Blazor uses for internal IDs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep good point, I just have an aversion to js startsWith for compatibility issues but I think that's an aversion I can now start to let go ^^ - this is included in the new commit

also changed outside click js check for _bl_ to be more specific
@HAL-NINE-THOUSAND
Copy link
Copy Markdown
Contributor Author

Hi Chris,

Thanks, I've committed the delay removal the better ID check

@chrissainty chrissainty left a 8000 comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Thank you @HAL-NINE-THOUSAND :shipit:

@chrissainty chrissainty added the Bug Something isn't working label Jan 7, 2021
@chrissainty
chrissainty merged commit 590589a into Blazored:main Jan 7, 2021
@HAL-NINE-THOUSAND
Copy link
Copy Markdown
Contributor Author

Glad I could help Chris, your libraries have been fantastic for learning Blazor and actually getting it out there in the wild

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] BlazoredTypeAhead inside BlazorStrap modal: Dropdown disappears when scrollbar is touched

3 participants

0