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

Commit

Permalink
Various bookmark favicon tweaks
Browse files Browse the repository at this point in the history
Most of the changes are because js/compontent/contextMenu.js should be a general component and not know anything about bookmarks or favicons.

I posted #1469 to add folder icons, I won't have time to focus on that
soon, in case you want to take it.

Fix #1467

Auditors: @bsclifton
  • Loading branch information
bbondy committed Apr 25, 2016
1 parent e7a93e9 commit 16dfefc
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 21 deletions.
2 changes: 1 addition & 1 deletion js/components/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class BookmarkToolbarButton extends ImmutableComponent {
{
!this.isFolder && showFavicon ? <span className='bookmarkFavicon' style={iconStyle}></span> : null
}
<span>
<span className='bookmarkText'>
{
this.props.bookmark.get('customTitle') || this.props.bookmark.get('title') || this.props.bookmark.get('location')
}
Expand Down
39 changes: 22 additions & 17 deletions js/components/contextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ const windowActions = require('../actions/windowActions')
const config = require('../constants/config')
const siteSettings = require('../state/siteSettings')
const cx = require('../lib/classSet.js')
const settings = require('../constants/settings')
const getSetting = require('../settings').getSetting

export default class ContextMenuItem extends ImmutableComponent {
get submenu () {
Expand Down Expand Up @@ -94,25 +92,22 @@ export default class ContextMenuItem extends ImmutableComponent {
return ''
}
render () {
let showFavicon = getSetting(settings.SHOW_BOOKMARKS_TOOLBAR_FAVICON) === true
const iconSize = 16
let iconStyle = {
minWidth: iconSize,
width: iconSize
}

if (showFavicon) {
let icon = this.props.contextMenuItem.get('favicon')

if (icon) {
iconStyle = Object.assign(iconStyle, {
backgroundImage: `url(${icon})`,
backgroundSize: iconSize,
height: iconSize
})
} else {
showFavicon = false
}
const icon = this.props.contextMenuItem.get('icon')
let faIcon
if (icon) {
iconStyle = Object.assign(iconStyle, {
backgroundImage: `url(${icon})`,
backgroundSize: iconSize,
height: iconSize
})
} else {
faIcon = this.props.contextMenuItem.get('faIcon')
}

if (this.props.contextMenuItem.get('type') === 'separator') {
Expand All @@ -133,7 +128,9 @@ export default class ContextMenuItem extends ImmutableComponent {
}
return <div className={cx({
contextMenuItem: true,
checkedMenuItem: this.props.contextMenuItem.get('checked')
hasFaIcon: faIcon,
checkedMenuItem: this.props.contextMenuItem.get('checked'),
hasIcon: icon || faIcon
})}
role='listitem'
draggable={this.props.contextMenuItem.get('draggable')}
Expand All @@ -152,7 +149,15 @@ export default class ContextMenuItem extends ImmutableComponent {
: null
}
{
!this.hasSubmenu && showFavicon ? <span className='bookmarkFavicon' style={iconStyle}></span> : null
!this.hasSubmenu && (icon || faIcon)
? <span className={cx({
contextMenuIcon: true,
hasFaIcon: !!faIcon,
fa: faIcon,
[faIcon]: !!faIcon
})}
style={iconStyle}></span>
: null
}
<span className='contextMenuItemText'
data-l10n-id={this.props.contextMenuItem.get('l10nLabelId')}>{this.props.contextMenuItem.get('label')}</span>
Expand Down
4 changes: 3 additions & 1 deletion js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,15 @@ function showBookmarkFolderInit (allBookmarkItems, parentBookmarkFolder, activeF
}

function bookmarkItemsInit (allBookmarkItems, items, activeFrame) {
let showFavicon = getSetting(settings.SHOW_BOOKMARKS_TOOLBAR_FAVICON) === true
return items.map((site) => {
const isFolder = siteUtil.isFolder(site)
const templateItem = {
bookmark: site,
draggable: true,
label: site.get('customTitle') || site.get('title') || site.get('location'),
favicon: site.get('favicon'),
icon: showFavicon ? site.get('favicon') : undefined,
faIcon: showFavicon && !site.get('favicon') ? 'fa-file-o' : undefined,
contextMenu: function (e) {
onBookmarkContextMenu(site, activeFrame, e)
},
Expand Down
6 changes: 5 additions & 1 deletion less/bookmarksToolbar.less
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
}
}
&.showFavicon {
height: 30px;
height: 28px;
}
.bookmarkToolbarButton {
border-radius: @borderRadius;
Expand Down Expand Up @@ -61,6 +61,10 @@
display: inline-block;
margin-right: 4px;
}
.bookmarkText {
text-overflow: ellipsis;
overflow: hidden;
}
.bookmarkFolderChevron {
color: #676767;
margin-left: 4px;
Expand Down
9 changes: 8 additions & 1 deletion less/contextMenu.less
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,20 @@
}
}

.bookmarkFavicon {
.contextMenuIcon {
font-size: 14px;
margin-right: 8px;
}
}

.contextMenuItem {
padding: 6px 10px 6px 20px;
&.hasIcon {
padding-left: 10px;
&.hasFaIcon {
padding-left: 12px;
}
}
&.checkedMenuItem {
padding-left: 4px;

Expand Down

0 comments on commit 16dfefc

Please sign in to comment.