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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
add tooltip for number of authors #7207
add tooltip for number of authors #7207
Conversation
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
Codecov Report
@@ Coverage Diff @@
## master #7207 +/- ##
======================================
Coverage 81.5% 81.5%
======================================
Files 97 97
Lines 5602 5602
======================================
Hits 4566 4566
Misses 1036 1036 |
@jywarren @nstjean @anthony-zhou kindly review :) Thanks! |
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.
@priyanka-choubey very nice 馃憤
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.
This looks good. I mentioned one small fix in this review, please take a look! Thanks.
app/views/wiki/_header.html.erb
Outdated
@@ -31,7 +31,7 @@ | |||
|
|||
<div style="padding-top:8px;text-align:center"> | |||
<span class="d-none d-lg-inline"> | |||
<i style="color:#888;" class="fa fa-user"></i> <%= number_with_delimiter(@node.authors.length) %> | | |||
<span rel="tooltip" title="The number of authors for this page." data-placement="top"><i style="color:#888;" class="fa fa-user"></i> <%= number_with_delimiter(@node.authors.length) %></span> || |
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 think you may have added an extra vertical bar ("|") after the tooltip.
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 think there should only be one.
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.
@anthony-zhou good catch. There should be only one vertical bar.
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.
@priyanka-choubey please do the changes that @anthony-zhou requested. Thanks.
ebd1108
to
87e1d68
Compare
87e1d68
to
c578b61
Compare
@jywarren @anthony-zhou can you kindly review? Thanks :v |
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.
@priyanka-choubey could you please post a screenshot of your change? Thanks.
@VladimirMikulic screenshot is attached in pr description :) |
@priyanka-choubey it's still the old screenshot :) |
@VladimirMikulic ohh sorry about that 馃槄 updated pr description |
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 good to me, thanks!
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.
Congrats on merging your first pull request! 馃檶馃帀鈿★笍 Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 馃檶 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 馃槙 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 馃槃 馃帀 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
This is super. Thanks a ton, and great work! 馃帀 |
Fixes #7170
app/views/wiki/_header.html.erb modified to add a tooltip for number of authors.
rake test
@publiclab/reviewers
for help, in a comment belowScreenshots attached