[Docs] Added icon for external links in nav #6090
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
We may want to keep the markdown pages around in order to avoid breaking links. @zpao? |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
joecritch
force-pushed
the
docs-external-link-icon
branch
from
de85f7d
to
efcc3fa
Compare
|
@jimfb Fair point! I've updated the commit to keep them in. We can deal with cleanup as a separate concern. |
|
@joecritch updated the pull request. |
| @@ -260,6 +260,22 @@ h1, h2, h3, h4, h5, h6 { | |||
| &.active { | |||
| color: $primary; | |||
| } | |||
|
|
|||
| &[href*="http"] { | |||
There was a problem hiding this comment.
Can we just add a class to these links instead?
react/docs/_plugins/sidebar_item.rb
Lines 6 to 7 in de09e0a
There was a problem hiding this comment.
@zpao Sure, Updated. If the plugin sees an href attribute, it'll add an "external" class.
…Tools and Examples in favor of this icon
joecritch
force-pushed
the
docs-external-link-icon
branch
from
efcc3fa
to
8886cee
Compare
|
@joecritch updated the pull request. |
|
Seems good. Where did the icon come from (want to make sure we have appropriate permissions)? |
|
@zpao Cool. It's from scratch. Sent you the PSD. |
|
Ok, I'm cool with this, let's ship it. |
[Docs] Added icon for external links in nav
|
Thanks @joecritch! |
| if item["href"] | ||
| classes.push("external") | ||
| end | ||
| className = classes.size ? "class=\"#{classes.join(' ')}\"" : ""; |
There was a problem hiding this comment.
This isn't right, here's what gets generated for empty items. I have another thing I want to change so going to follow up.
<li>
- <a href="/react/docs/webcomponents.html">Web Components</a>
+ <a href="/react/docs/webcomponents.html"class="">Web Components</a>
</li>There was a problem hiding this comment.
Hmm, I'm getting a different output locally (ruby 2.2)
There was a problem hiding this comment.
bundle exec? It shouldn't really be a bundler issue though because 0 isn't falsey in Ruby.
There was a problem hiding this comment.
Ah shoot, didn't realise I needed to re-boot bundle exec jekyll after each plugin change. Hence the false positive. Sorry for the mess.
There was a problem hiding this comment.
No worries :) #6151 in case you're interested
[Docs] Added icon for external links in nav (cherry picked from commit 8f2b7d8)
Screenshot
Note
The icon was designed from scratch.