Modify subscription links component #294
Conversation
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
386aad3 to
fc75a6d
Compare
andysellick
changed the title
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
fc75a6d to
4f6993c
Compare
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
4f6993c to
cbe3988
Compare
andysellick
changed the title
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
cbe3988 to
ca2a61e
Compare
|
|
||
| <% if feed_link_box_value %> | ||
| <div class="gem-c-subscription-links__feed-box js-hidden" id="feed-reader"> | ||
| <p class="gem-c-subscription-links__feed-description js-hidden"><%= feed_link_text %></p> |
There was a problem hiding this comment.
It looks like the js-hidden means that this is never shown?
See this example in the component guide: https://govuk-publishing-compon-pr-294.herokuapp.com/component-guide/subscription-links/with_copyable_feed_link/preview
There was a problem hiding this comment.
If Javascript is disabled, it gets shown and the main feed link gets hidden. Seemed like a sensible way to keep the text but remove the link/button.
| } | ||
|
|
||
| .gem-c-subscription-links__feed-description { | ||
| font-weight: bold; |
There was a problem hiding this comment.
Should we be using our font mixins here?
There was a problem hiding this comment.
I think the only way to do that is to specify a font size as well? I deliberately didn't, because I didn't want to set a font size, only the weight.
| @@ -1,21 +1,50 @@ | |||
| <% | |||
| email_signup_link ||= false | |||
| email_signup_link_text ||= "Get email alerts" | |||
There was a problem hiding this comment.
Should this and the default feed link text be in locale files for translation?
There was a problem hiding this comment.
Not a bad idea, will add.
| <% if email_signup_link || feed_link %> | ||
| <section class="gem-c-subscription-links"> | ||
| <% if email_signup_link || feed_link || feed_link_box_value %> | ||
| <section class="gem-c-subscription-links" data-module="gem-toggle"> | ||
| <h2 class="visuallyhidden">Subscriptions</h2> |
There was a problem hiding this comment.
Same here for using translated string
| feed_link_text: 'View feed' | ||
| with_copyable_feed_link: | ||
| description: | | ||
| This option changes the feed link to a toggle control, which opens a hidden element containing an input containing the value passed to the component, usually a URL to an atom feed. This uses the [form input](/component-guide/input) component. |
There was a problem hiding this comment.
Nitpick: this reads a bit weirdly "..which opens a hidden element containing an input containing the value...".
Could we say "..which opens a hidden element containing an input prepopulated with the value...
There was a problem hiding this comment.
Much better, thanks.
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
ca2a61e to
41c7d53
Compare
- add option to change link text - add option to turn feed link into a toggle to display an input box containing a link - includes toggle.js from static, included without changes
andysellick
force-pushed
the
iterate-subscription-links-component
branch
from
41c7d53 to
0ddaaa2
Compare
| feed_link ||= false | ||
| feed_link_text ||= t("govuk_component.subscription_links.feed_link_text", default: "Subscribe to feed") | ||
| feed_link_box_value ||= false |
There was a problem hiding this comment.
This is way too late (again), but will feed_link_box_value ever be different from feed_link? Perhaps a boolean param to show the box or not would make it easier to use.
The subscription links component is going to be used on the new organisation pages, but needs a few extra features adding.
Component guide link: https://govuk-publishing-compon-pr-294.herokuapp.com/component-guide/subscription-links
Trello card: https://trello.com/c/NjgS3vym/55-modify-component-subscription-links