8000 Support customized shouldFollowLink method by MattHo · Pull Request #80 · yahoo/react-i13n · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support customized shouldFollowLink method #80

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 9, 2016
Merged

Support customized shouldFollowLink method #80

merged 1 commit into from
Mar 9, 2016

Conversation

MattHo
Copy link
Contributor
@MattHo MattHo commented Mar 7, 2016

@kaesonho

This is to enable component to have customized followLink behavior.

@kaesonho
Copy link
Contributor
kaesonho commented Mar 7, 2016

@redonkulus @lingyan to allow followLink value as dynamic, we should provide a feature to allow users to pass props.followLink or props.follow as a function, however for most case this mechanism needs an additional wrapper to read value from props also some additional logic then pass it as a function. to prevent this we allow user to implement shouldFollowLink directly in their component which is using I13nMixin

@yahoocla
Copy link
yahoocla commented Mar 7, 2016

CLA is valid!

kaesonho
kaesonho reviewed Mar 7, 2016
View reviewed changes
src/mixins/I13nMixin.js
* @method _shouldFollowLink
* @private
*/
_shouldFollowLink: function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to clickHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I put it in mixin is to leave document for users to know they can implement shouldFollowLink() method.

Cop 8000 y link
Contributor

Choose a reason for hiding this comment

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

as we discussed, we don't encourage users to use I13nMixin directly, (use createI13nNode instead),
so there's no document for I13nMixin, I'm ok with this if we all agree this is clear enough as a document

@MattHo MattHo mentioned this pull request Mar 8, 2016
Merged
@kaesonho
Copy link
Contributor
kaesonho commented Mar 8, 2016

👍 test failed though

kaesonho added a commit that referenced this pull request Mar 9, 2016
@kaesonho
Merge pull request #80 from MattHo/shouldFollowLink
ddb4447 
Support customized shouldFollowLink method
@kaesonho kaesonho merged commit ddb4447 into yahoo:master Mar 9, 2016
@MattHo MattHo deleted the shouldFollowLink branch March 9, 2016 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MattHo @kaesonho @yahoocla
0