-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add components for Article #24
Conversation
|
||
module Base | ||
module Button | ||
class Component < ApplicationComponent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has 7 constants |
Generated by 🚫 Danger |
@options = options | ||
|
||
@options[:tag] ||= :a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering a few days ago how to have a clean solution that accepts both <button>
and <a>
tags :)
That way is nice!
One question though: If the tag is 'button', should we remove the default value of the link
param?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Will do that,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment 👆, overall LGTM :)
Thanks a lot for that one, it gives a much better structure to components~ I'll start from this branch for login Font-End ;-)
@rafayet-monon please pull the last commit on your local branch (d1c587d), I've added minor changes so that Unit Tests passes ;-) |
…/rails-view-component-blog into feature/articles-view-component
|
||
private | ||
|
||
def classes(type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't depend on instance state (maybe move it to another class?) |
This PR partially does some tickets component part-
What happened
Add components in articles
Insight
Added components for
Proof Of Work
Test cases should pass