Hide podcast app launchers on desktop #414
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have had customer feedback that these buttons don't do anything on desktop and it is a concern that we are losing out on opportunities for engagement by showing people broken buttons. This commit: - Uses a media query to hide the app launcher buttons on desktop as they don't work - Introduces an oGrid dependency to get the standard breakpoint for desktop
alicebartlett
temporarily deployed
to
x-dash-fix-podcast-laun-x8kjaf
|
LGTM @alicebartlett |
|
@robsquires good tip Rob - I'll check that |
|
OK - I've checked this on next-article and it looks good. @robsquires is gonna check on the app so I don't have to get all that set up, and then I think we're good! |
|
@alicebartlett we've lost the buttons on a landscape ipad 😢
|
As part of a bug raised via editorial, we need to hide the podcast app links on desktop because they don't do anything. To do this I have used a combination of @media query to show at narrow screens (ie when the website is viewed on a mobile) and an isApp prop to always show it if you're viewing in the app wrapper.
|
Woop! 🎯
|
robsquires
previously approved these changes
Jan 9, 2020
robsquires
reviewed
Jan 9, 2020
| @@ -48,39 +48,38 @@ class PodcastLaunchers extends Component { | |||
|
|
|||
| render() { | |||
| const { rssUrl } = this.state; | |||
| const { conceptId, conceptName, csrfToken, isFollowed, renderFollowButton } = this.props | |||
| const { conceptId, conceptName, csrfToken, isFollowed, renderFollowButton, showLinksOnAllBreakpoints } = this.props | |||
There was a problem hiding this comment.
As per IRL chat, default showLinksOnAllBreakpoints to true
robsquires
previously approved these changes
Jan 23, 2020
|
🚢👈 |
apaleslimghost
approved these changes
Sep 16, 2020
There was a problem hiding this comment.
sorry this PR got forgotten about! it looks good now 🙂 @robsquires, since Alice is away would you mind shepherding this through to release (if it's still needed)?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, this is my first foray into x-dash, so it's very possible I've done something obvious incorrectly. Please be vigilant when reviewing this PR!
We have had customer feedback that these buttons don't do anything on
desktop and it is a concern that we are losing out on opportunities
for engagement by showing people broken buttons.
This commit:
they don't work
desktop
If this is your first
x-dashpull request please familiarise yourself with the contribution guide before submitting.I don't know how x-dash manages Origami dependencies. This is either a patch release because it is a very small change that won't break anything, or it is major because I have introduced a new Origami dependency which could break a dependency tree somewhere. Can anyone advise on this?
✅ Discuss features first (chatted with @robsquires)
✅ Update the documentation
✅ No hacks, experiments or temporary workarounds
? Decide on a version (major, minor, or patch)