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

Added release notes link to about:brave #7791

Merged
merged 2 commits into from
Apr 4, 2017
Merged

Added release notes link to about:brave #7791

merged 2 commits into from
Apr 4, 2017

Conversation

srirambv
Copy link
Collaborator

@srirambv srirambv commented Mar 20, 2017

Test Plan

  1. Open about:brave
  2. Should contain Browse faster and safer with Brave. below About Brave
  3. Should have a new section Release Notes
  4. Clicking on hyperlink should open GitHub release page

Description

  • 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).

app.properties file alphabetized for easier readability and future new additions, new values for the PR in line 35, 177-180

Closes: #6130

@bsclifton
Copy link
Member

@srirambv can you resolve the conflicts? This just needs a rebase. I can review in the meantime and then accept quickly if everything looks good 😄 thanks!

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.

Per the spec (the issue, screenshot)- there should be a link to a specific version (or tag) of Brave. If we always link to the latest version, people several versions from now may mistakenly think they're on the latest

Trying to find this might not be easy- we definitely have the hash available... but we'll need to do a lookup (AJAX call, etc) to find a tag with that associated SHA.

@srirambv
Copy link
Collaborator Author

@bsclifton conflicts resolved

@srirambv
Copy link
Collaborator Author

Changed href to use the installed ver to open the release notes page. Thanks to @NejcZdovc for the help

@bsclifton bsclifton modified the milestones: 0.14.0, 0.14.1 Mar 21, 2017
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.

Changes look good to me!

I wanted to add Brad as a reviewer here, since there is an update to the copy (ex: it now says About Brave - Browse faster and safer with Brave.) and the look. We could use this opportunity to put a Lion logo in there (I can help with this if needed)

Here's a screenshot of the new UI:
screen shot 2017-04-03 at 1 10 52 am

@bradleyrichter
Copy link
Contributor

"Release Notes" should get the same style as "Version Information" because "About Brave" should be the primary title.

I don't think we need a lion icon here, and it would upset the alignment unless we make it a 2 column layout.

@NejcZdovc NejcZdovc removed their request for review April 3, 2017 21:39
@NejcZdovc
Copy link
Contributor

@srirambv Link implementation looks good

Copy link
Contributor

@luixxiul luixxiul left a comment

Choose a reason for hiding this comment

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

I would agree with @bradleyrichter. Except that, LGTM.

srirambv and others added 2 commits April 4, 2017 11:22
@bsclifton
Copy link
Member

I made a quick commit w/ the feedback 😄
screen shot 2017-04-04 at 11 20 02 am

@srirambv
Copy link
Collaborator Author

srirambv commented Apr 5, 2017

Thanks for the quick update @bsclifton 👍

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