-
Notifications
You must be signed in to change notification settings - Fork 212
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
#621 Crop varieties sidebar #729
Conversation
visit crop_path(crop) | ||
|
||
# It lists all 5 items (note: including the top level item.) | ||
# It DOES NOT have "Show all/less" toggle link |
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.
You might use a within
block to focus on that .crop-hierarchy class.
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.
Good suggestion, thanks! Done now.
b29a9f5
to
c253c86
Compare
1 similar comment
Code has been updated to incorporate @tygriffin's review and also
Apologize for modifying some stuff after the review. Can you please review this again? |
Looks good to me - moving into release 9. |
Please review.
Implemented #621 with javascript as I didn't realize rails-y implementation was preferred over javascript here and I wanted the toggle to look smooth instead of refreshing the whole page.
Let me know if you want me to rewrite using different technology etc.
Thanks!