Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Advertise community.brave.com #6080

Merged
merged 1 commit into from
Dec 13, 2016
Merged

Advertise community.brave.com #6080

merged 1 commit into from
Dec 13, 2016

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Dec 8, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Auditors: @bsclifton, @alexwykoff

Test Plan:

  1. Make sure the anchor link to the community (Discourse) on README works
  2. Make sure each of these links opens the community in a new tab
    • "Submit Feedback..." in Help on the menu bar
    • "Report an issue" in Help on the menu bar
    • "Submit Feedback..." in Help on the hamburger menu
    • "Report an issue" in Help on the hamburger menu
    • "Submit Feedback..." on about:preferences

@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 8, 2016

Some details should be added on README.md.

@alexwykoff
Copy link
Contributor

Please don't forget the hamburger (now kabob) menu links.
'Help' -> 'Report an Issue' should link to community as well for now.

A spec is in progress for the new help actions, but this is a good stopgap. Thanks for getting it started!

@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 8, 2016

@alexwykoff then you would have the two items in the same submenu which are linked to the same page, which is not expected? In this case either of it should be removed.

@alexwykoff
Copy link
Contributor

You could deep link to the appropriate threads in community, but yes, I did suggest double-linking because it is a stopgap, not a full solution.

@@ -10,7 +10,8 @@ const Immutable = require('immutable')
const locale = require('../../js/l10n')
const settings = require('../../js/constants/settings')
const getSetting = require('../../js/settings').getSetting
const issuesUrl = 'https://github.com/brave/browser-laptop/issues'
// const issuesUrl = 'https://github.com/brave/browser-laptop/issues'
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest just deleting this; comments usually just end up cluttering and we can always look at the previous revision 😄

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

++

@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 9, 2016

@bsclifton would you please add information about the community on README.md which sounds natural to native speakers?

@luixxiul luixxiul added this to the 0.13.1 milestone Dec 11, 2016
@luixxiul
Copy link
Contributor Author

Note: reportAnIssueMenuItem and submitFeedbackMenuItem in commonMenu.js and menu.js should be merged and replaced later

- Added the link to the community on README
- Replaced the link to GitHub issues page with "community.brave.com"
- Removed issuesUrl
- Removed APP_SUBMIT_FEEDBACK
- data-l10n-id in preferences.properties was replaced from 'sendUsFeedback' to 'submitFeedback', which has been existed in menu.properties
  TODO: 'reportAnIssue' and 'submitFeedback' will be merged later

Auditors: @bsclifton, @alexwykoff

Test Plan:
1. Make sure the anchor link to the community (Discourse) on README works
2. Make sure each of these links opens the community in a new tab
  - "Submit Feedback..." in Help on the menu bar
  - "Report an issue" in Help on the menu bar
  - "Submit Feedback..." in Help on the hamburger menu
  - "Report an issue" in Help on the hamburger menu
  - "Submit Feedback..." on about:preferences
@alexwykoff
Copy link
Contributor

lgtm!

@bbondy bbondy merged commit 3f09030 into brave:master Dec 13, 2016
@bbondy bbondy modified the milestones: 0.12.15, 0.13.1 Dec 13, 2016
@bbondy
Copy link
Member

bbondy commented Dec 13, 2016

Added an issue for it here:
#6179

@luixxiul
Copy link
Contributor Author

@bbondy thanks for doing that!

@luixxiul luixxiul changed the title [WiP] Advertise community.brave.com Advertise community.brave.com Dec 13, 2016
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

++ 😄

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.

5 participants