-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Adding the links below the graph for Tags page #5209
Conversation
Generated by 🚫 Danger |
The image looks so close to the aim.. 😍 |
@jywarren Kindly look into the PR and merge if all seem fine. Thanks! |
app/views/tag/index.html.erb
Outdated
<a style="cursor: pointer;" onclick="document.getElementById('myImage').src='stats/graph.html?limit=100'"><u>100</u></a> | | ||
<a style="cursor: pointer;" onclick="document.getElementById('myImage').src='stats/graph.html?limit=250'"><u>250</u></a> | | ||
<a style="cursor: pointer;" onclick="document.getElementById('myImage').src='stats/graph.html?limit=500'"><u>500</u></a> | ||
</h6> |
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.
Please indent the code.
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.
wow this is such a clever solution! Thank you!
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.
Thank you @jywarren @gauravano!
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.
Looks great! Could we also highlight the link currently active?
@gauravano sure i'll do it. Thanks! |
If it's possible to highlight the link and you can share a screenshot of that working, we're happy to merge this, thanks! |
Okay @jywarren. I'll highlight the active link and make a PR. Should I make a different PR or push commit here? |
@jywarren @gauravano I have implemented highlighting the active link and pushed commit to this PR. Kindly look into it. Thanks! |
This is so nice!!! I'm merging now. Great work! |
Thanks @jywarren |
Ok, done! Thanks!
…On Thu, Mar 21, 2019, 4:45 PM Gautami Gupta ***@***.***> wrote:
Thanks @jywarren <https://github.com/jywarren>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5209 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ2LRaLKjEVIdd3OhLEQJpLkf2Wa8ks5vY-86gaJpZM4cAT_g>
.
|
* add_links_below_graphs * add_links_below_graphs2 * highlighting the active link
* add_links_below_graphs * add_links_below_graphs2 * highlighting the active link
* add_links_below_graphs * add_links_below_graphs2 * highlighting the active link
Fixes #5090 (<=== Add issue number here)
Fixing the 3rd task of adding links below the graph for the new design of the tags page.
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!