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

An option to pass mixin options on inherting a mixin. (within the tag instance and/or global mixin) #2434

Closed
chardskarth opened this issue Aug 30, 2017 · 8 comments

Comments

@chardskarth
Copy link

@chardskarth chardskarth commented Aug 30, 2017

An option to pass mixin options on inherting a mixin. (within the tag instance and/or global mixin)

  1. Describe your issue:
    I initially thought of having a generic mixin where I could reuse it across different tag instances while configuring some functionality of each mixin inheritance. Is this design usable? Or does it have its caveats enough not to implement this design?

  2. Can you reproduce the issue?
    Here's a sample on how I imagine to use thisl.

  3. On which browser/OS does the issue appear?
    N/A

  4. Which version of Riot does it affect?
    N/A

  5. How would you tag this issue?

  • Question
  • Feature request
  • Enhancement
  • Bug
  • Discussion
  • Tip
  • Performance
@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Aug 30, 2017

You can read always your tag options via this.opts what about this solution?

@chardskarth
Copy link
Author

@chardskarth chardskarth commented Aug 30, 2017

@GianlucaGuarini I was thinking about not crowding up the tag instance (too much) space with data that only a specific mixin should worry about. Was thinking about this design for mixins

GianlucaGuarini added a commit that referenced this issue Sep 2, 2017
@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Sep 2, 2017

@chardskarth good idea, in the next riot release the mixins will receive the tag options via init method

@chardskarth
Copy link
Author

@chardskarth chardskarth commented Sep 4, 2017

@GianlucaGuarini You are awesome! Riot is promising. 👍 😄

@chardskarth
Copy link
Author

@chardskarth chardskarth commented Sep 4, 2017

@GianlucaGuarini Actually, I think you misunderstood. You passed the tag instance options. I was talking about mixin options.

chardskarth pushed a commit to chardskarth/riot that referenced this issue Sep 4, 2017
fix GianlucaGuarini@8c1f3e90c7082f06ba6ad59cc349dee95b6d91a5
chardskarth added a commit to chardskarth/riot that referenced this issue Sep 4, 2017
fix GianlucaGuarini@8c1f3e90c7082f06ba6ad59cc349dee95b6d91a5
chardskarth added a commit to chardskarth/riot that referenced this issue Sep 4, 2017
fix GianlucaGuarini@8c1f3e90c7082f06ba6ad59cc349dee95b6d91a5
@syuilo
Copy link
Contributor

@syuilo syuilo commented Nov 11, 2017

+1 for @chardskarth.
I have the same idea. Will this be implemented in the future?

@chardskarth
Copy link
Author

@chardskarth chardskarth commented Nov 12, 2017

Yeah, I hope my pull request gets verified soon.

@syuilo
Copy link
Contributor

@syuilo syuilo commented Nov 12, 2017

Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.