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

Reordered Hamburger Menu with layout mentioned in #1893 #3003

Merged
merged 3 commits into from
Aug 18, 2016
Merged

Reordered Hamburger Menu with layout mentioned in #1893 #3003

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
288310c
Reordered Hamburger Menu with layout mentioned in #1893
Aug 6, 2016
4edc1e0
Updated Hamburger to included initial required items
Aug 11, 2016
cdae492
Removed unrelated changes
Aug 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions js/components/navigationBar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,6 @@ class NavigationBar extends ImmutableComponent {
})}
onClick={this.onNoScript} />
}
<Button iconClass={this.titleMode ? 'fa-star' : 'fa-star-o'}
className={cx({
navbutton: true,
bookmarkButton: true,
removeBookmarkButton: this.bookmarked
})}
l10nId={this.bookmarked ? 'removeBookmarkButton' : 'addBookmarkButton'}
onClick={this.onToggleBookmark} />
</div>
}
</div>
Expand Down
9 changes: 9 additions & 0 deletions js/components/urlBar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const windowStore = require('../stores/windowStore')
var searchProviders = require('../data/searchProviders')
const searchIconSize = 16
const UrlUtil = require('../lib/urlutil')
const Button = require('./button')

const { isUrl, isIntermediateAboutPage } = require('../lib/appUrlUtil')

Expand Down Expand Up @@ -368,6 +369,14 @@ class UrlBar extends ImmutableComponent {
action='#'
id='urlbar'
ref='urlbar'>
<Button iconClass={this.titleMode ? 'fa-star' : 'fa-star-o'}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand, why was bookmarks moved into the urlbar component? It seems unrelated to the rest of the changeset.

Copy link
Author

Choose a reason for hiding this comment

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

It is, It should be on another branch, not sure how it slipped in. This change is to tackle the new bookmark design.

className={cx({
navbutton: true,
bookmarkButton: true,
removeBookmarkButton: this.bookmarked
})}
l10nId={this.bookmarked ? 'removeBookmarkButton' : 'addBookmarkButton'}
onClick={this.onToggleBookmark} />
<span
onDragStart={this.onDragStart}
draggable
Expand Down
27 changes: 11 additions & 16 deletions js/contextMenus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,19 +533,10 @@ function getEditableItems (selection, editFlags) {

function hamburgerTemplateInit (location, e) {
const template = [
{
l10nLabelId: 'new',
submenu: [
CommonMenu.newTabMenuItem(),
CommonMenu.newPrivateTabMenuItem(),
CommonMenu.newPartitionedTabMenuItem(),
CommonMenu.separatorMenuItem,
CommonMenu.newWindowMenuItem()
]
},
CommonMenu.separatorMenuItem,
CommonMenu.findOnPageMenuItem(),
CommonMenu.printMenuItem(),
CommonMenu.newTabMenuItem(),
CommonMenu.newPrivateTabMenuItem(),
CommonMenu.newPartitionedTabMenuItem(),
CommonMenu.newWindowMenuItem(),
CommonMenu.separatorMenuItem,
{
l10nLabelId: 'zoom',
Expand All @@ -569,7 +560,6 @@ function hamburgerTemplateInit (location, e) {
}]
},
CommonMenu.separatorMenuItem,
CommonMenu.preferencesMenuItem(),
{
label: locale.translation('bookmarks'),
submenu: [
Expand All @@ -578,14 +568,18 @@ function hamburgerTemplateInit (location, e) {
CommonMenu.separatorMenuItem,
CommonMenu.importBookmarksMenuItem()
]
}, {
},{
label: locale.translation('bravery'),
submenu: [
CommonMenu.braveryGlobalMenuItem(),
CommonMenu.braverySiteMenuItem()
]
},
CommonMenu.downloadsMenuItem(),
CommonMenu.findOnPageMenuItem(),
CommonMenu.printMenuItem(),
CommonMenu.separatorMenuItem,
CommonMenu.preferencesMenuItem(),
CommonMenu.separatorMenuItem,
Copy link
Member

Choose a reason for hiding this comment

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

we should still prefer the Bravery menu.

CC @bradleyrichter which order would you like it in?

{
label: locale.translation('help'),
Expand All @@ -597,7 +591,8 @@ function hamburgerTemplateInit (location, e) {
CommonMenu.reportAnIssueMenuItem(),
CommonMenu.submitFeedbackMenuItem()
]
}
},
CommonMenu.quitMenuItem()
Copy link
Member

Choose a reason for hiding this comment

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

Ditto I don't think we should get rid of about brave and check for updates.
CC @bradleyrichter

]
return template
}
Copy link
Member

Choose a reason for hiding this comment

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

@brad do you want report an issue and submit feedback removed now?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep one of them. It promotes teamwork. : )

Chrome has Report and Issue. FX has Submit feedback. Any pref @bbondy ?

Copy link
Author

Choose a reason for hiding this comment

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

@bbondy @bradleyrichter I believe I misinterpreted the ticket, My mistake, excuse that

Expand Down