Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

feat(topstories): Hide preference section for unsupported locales #3002

Merged
merged 1 commit into from
Jul 27, 2017

Conversation

csadilek
Copy link
Collaborator

@csadilek csadilek commented Jul 27, 2017

Suggestion for a simple fix #2973.

Reason I used "hidden" (negated showByDefault) is so nobody has to reset their prefs to keep seeing top stories.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.407% when pulling fb5504d on csadilek:2973 into d54be96 on mozilla:master.

Copy link
Member

@Mardak Mardak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat. I was worried that we needed to have some new common/constants.jsm or something.

const SECTIONS = new Map([
["topstories", {
feed: TopStoriesFeed,
prefTitle: "Fetches content recommendations from a configurable content provider",
// for now, we only want to show top stories by default to the following locales
showByDefault: ["en-US", "en-CA"].includes(Services.locale.getRequestedLocale())
showByDefault: showTopStoriesByDefault
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the default inline with topstories, i.e., undo this change. So that the comment line makes sense.

@@ -84,7 +85,8 @@ const PREFS_CONFIG = new Map([
"api_key_pref": "extensions.pocket.oAuthConsumerKey",
"provider_name": "Pocket",
"provider_icon": "pocket",
"provider_description": "pocket_feedback_body"
"provider_description": "pocket_feedback_body",
"hidden": ${!showTopStoriesByDefault}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead just do ${!SECTIONS.get("topstories").showByDefault}

@@ -24,13 +24,14 @@ const {TopStoriesFeed} = Cu.import("resource://activity-stream/lib/TopStoriesFee

const REASON_ADDON_UNINSTALL = 6;

// For now, we only want to show top stories by default to the following locales
const showTopStoriesByDefault = ["en-US", "en-CA"].includes(Services.locale.getRequestedLocale());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine with the comment line moved up.

@Mardak Mardak merged commit edb5f04 into mozilla:master Jul 27, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.407% when pulling 1fede7c on csadilek:2973 into d54be96 on mozilla:master.

@as-pine-proxy
Copy link
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide Stories/Pocket preference section for locales that have it off by default
4 participants