Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Improve urth-core-behaviors/error-display-behavior #555

Closed

Conversation

haobibo
Copy link
Contributor

@haobibo haobibo commented Mar 28, 2017

Before: error message append into
After: error message append after .

This is particularly useful when users write
<urth-core-function><buton>invoke</button></urth-core-function>.
Before this improvements, when users click on the error message, the
function will be invoked again (click event broadcast to error message).
After this improvement, this issue is fixed.

Before: error message append into <urth-core-functoin>
After: error message append after <urth-core-function>.

This is particularly useful when users write
`<urth-core-function><buton>invoke</button></urth-core-function>`.
Before this improvements, when users click on the error message, the
function will be invoked again (click event broadcast to error message).
After this improvement, this issue is fixed.
@lbustelo
Copy link
Collaborator

I think this is fine... there are other elements that end up mixing in this behavior... @haobibo did you do a sniff test around all the other elements to make sure there is no obvious issues with writing outside the element's context?

@haobibo
Copy link
Contributor Author

haobibo commented Mar 30, 2017

@lbustelo It seems a lot work to do if we test error display behavior because many elements use this behavior.
Can we just do this improvement to urth-core-funciton? This commit ( 0d453b4 ) will only change the behavior for urth-core-function.

@lbustelo
Copy link
Collaborator

@haobibo I don't think it is ok for a behavior to have explicit knowledge of elements that may mix-in the code. error-display-behavior should know know anything about the existence of urth-core-function.

@haobibo
Copy link
Contributor Author

haobibo commented Jun 19, 2017

@lbustelo the latest commit allow urth-core-function to listen click events only for its children elements instead of itself.
However, the build tests failed with bower ECONFLICT Unable to find suitable version for test-fixture .

@lbustelo
Copy link
Collaborator

Sounds to me like dependency hell... Some dependency somewhere needs to be temporarily pegged or the tests need to be updated to support new versions.

@haobibo haobibo closed this by deleting the head repository Feb 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants