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

Removing clear cache and clear site data options from the history menu #5251

Merged
merged 1 commit into from
Oct 30, 2016

Conversation

maazadeeb
Copy link
Contributor

@maazadeeb maazadeeb commented Oct 29, 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).

Fix #3093
There were 3 options to clear history, namely Clear History, Clear Cache, Clear All Cookies and Site Data. These seemed redundant. Removing the 3 and keeping only one option called "Clear Browsing Data". The particular string seems to already exist in history.properties file. Added the same entry with the key "clearBrowsingData" in menu.properties as well.

Test Plan:

  1. Make sure only one option exists for clearing history, from history menu
    • Open Brave
    • Select the History menu option
    • See that only "Clear Browsing Data" option is available

2. Test option name in different supported languages
- Verify that the option name is correct in different languages

PS2: Removed the translation file changes
PS3: Changed "Clear browsing data..." to "Clear Browsing Data...", as requested in the issue

Auditors: @bsclifton

@bsclifton
Copy link
Member

Change looks good- the modification for other language files is not needed (more info here) 😄

@bbondy
Copy link
Member

bbondy commented Oct 30, 2016

Good to merge if you remove the translation changes other than en-US, thanks!
git reset head^ --soft
Then git add just the files you need and git checkout -- filename the ones you don't should do it.
Then you can just git push -f sorry if you already know all of this :)

@maazadeeb
Copy link
Contributor Author

I thought since the translation of the string is already available, and I only need to append ..., why not do that as well. 😄 But as per the link provided by @bsclifton I can see that you have a process regarding translations and I guess that'll take care of this. Will update the PR.

@bbondy Thanks for the git commands! I was just googling around how to update a PR 😄

@bsclifton
Copy link
Member

Here's the before:
screen shot 2016-11-04 at 2 01 42 pm

This change consolidates it down to one single menu:
"Clear Browsing Data…"

@maazadeeb maazadeeb deleted the single-history-clear branch December 8, 2016 05:37
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.

6 participants