Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brave Today: popup menu for publisher name which disables publisher #7104

Merged
merged 3 commits into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .storybook/locale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,8 @@ const locale: Record<string, string> = {
showTopSites: 'Show Top Sites',
showRewards: 'Show Rewards',
rewardsWidgetEnableBrandedWallpaperTitle: 'Get paid to view this sponsored background image.',
tosAndPp: 'By turning on {{title}}, you agree to the $1Terms of Service$2 and $3Privacy Policy$4.'
tosAndPp: 'By turning on {{title}}, you agree to the $1Terms of Service$2 and $3Privacy Policy$4.',
braveTodayDisableSourceCommand: 'Disable content from $1',
}

export default locale
2 changes: 2 additions & 0 deletions browser/ui/webui/brave_webui_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ void CustomizeWebUIHTMLSource(const std::string &name,
{ "braveTodayResetConfirm", IDS_BRAVE_TODAY_RESET_CONFIRM},
{ "braveTodayCategoryNameAll", IDS_BRAVE_TODAY_CATEGORY_NAME_ALL},
{ "braveTodaySourcesTitle", IDS_BRAVE_TODAY_SOURCES_TITLE},
{ "braveTodayDisableSourceCommand",
IDS_BRAVE_TODAY_DISABLE_SOURCE_COMMAND},

{ "addWidget", IDS_BRAVE_NEW_TAB_WIDGET_ADD },
{ "hideWidget", IDS_BRAVE_NEW_TAB_WIDGET_HIDE },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,9 @@ export const Heading = styled(StandardHeading)`
margin: 36px 0 24px;
`

export const Text = styled(StandardText)``
export const Text = styled(StandardText)`
${Intro} & {
font-weight: 500;
line-height: 20px;
}
`
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ import {
Heading as StandardHeading,
Image as StandardImage,
Text as StandardText,
Time as StandardTime,
PublisherLogo as StandardPublisherLogo
Time as StandardTime
} from './default'

export const Large = styled(StandardBlock.withComponent('article'))`
padding: 0;
`

export const Medium = styled(Large)`
export const Medium = styled(StandardBlock.withComponent('article'))`
padding: 0;
`

export const Small = styled(Large)`
export const Small = styled(StandardBlock.withComponent('article'))`
padding: 0;
width: 100%;
min-height: 310px;
`
Expand Down Expand Up @@ -74,7 +74,7 @@ export const Image = styled(StandardImage)`
`

export const Heading = styled(StandardHeading)`
font-weight: 500;
font-weight: 600;
a {
display: block;
color: inherit;
Expand All @@ -90,7 +90,7 @@ export const Text = styled(StandardText)`
font-family: ${p => p.theme.fontFamily.heading};
font-size: 18px;
line-height: 25px;
font-weight: 500;
font-weight: 600;

a {
display: block;
Expand All @@ -99,10 +99,18 @@ export const Text = styled(StandardText)`
}
`

export const Time = styled(StandardTime)``
export const Time = styled(StandardTime)`
${Large} & {
margin-top: 8px;
}
`

export const PublisherLogo = styled(StandardPublisherLogo)`
margin: 36px 0 12px;
export const Publisher = styled('div')`
box-sizing: border-box;
margin: 10px 0 0 0;
padding: 0;
font: 500 14px ${p => p.theme.fontFamily.heading};
color: #fff;
`

export const ContainerForTwo = styled<{}, 'div'>('div')`
Expand All @@ -114,7 +122,7 @@ export const ContainerForTwo = styled<{}, 'div'>('div')`

export const DealsCategory = styled('h3')`
margin: 0;
font: 700 18px ${p => p.theme.fontFamily.heading};
font: 600 18px ${p => p.theme.fontFamily.heading};
color: white;
`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,200 @@
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as React from 'react'
import * as Card from '../cardSizes'
import * as styled from '../default'
import styled from 'brave-ui/theme'
import { getLocale } from '../../../../../common/locale'
import { OnSetPublisherPref } from '../'

type Props = {
publisher: BraveToday.Publisher
onSetPublisherPref: OnSetPublisherPref
title?: boolean
}

export default function PublisherMeta (props: Props) {
const Component = props.title
? Card.PublisherHeading
: styled.PublisherName
// TODO(petemill): Make this shared and have WidgetMenu use it,
// so we're not duplicating styles, functionality and accessibility.
const isOpenClassName = 'is-open'

const PublisherMeta = styled('div')`
width: fit-content;
`

const Trigger = styled('button')`
appearance: none;
display: block;
cursor: pointer;
position: relative;
margin: 0;
border: none;
border-radius: 100px;
padding: 0;
background: none;
display: flex;
flex-direction: row;
justify-content: flex-start;
align-items: center;
outline: none;
color: inherit;
font-weight: inherit;

// Negative margin is ok here because we're doing a
// "ghost" outline, but we do need to take care not to overlap any sibling
// elements.
--ghost-padding-v: max(4.7%, 5px);
--ghost-padding-h: max(9%, 12px);
padding: var(--ghost-padding-v) var(--ghost-padding-h);
margin: calc(var(--ghost-padding-v) * -1 - 1px) calc(var(--ghost-padding-h) * -1 - 1px);
border: solid 1px transparent;
overflow: visible;

&.${isOpenClassName},
&:focus-visible,
&:hover,
&:active {
border-color: inherit;
}

&:active {
background-color: rgba(255, 255, 255, .2);
}
`

const Text = styled('span')`
max-width: 100%;
overflow: hidden;
text-overflow: ellipsis;
`

const Menu = styled('ul')`
list-style: none;
list-style-type: none;
margin: 0;
position: absolute;
width: max-content;
min-width: 166px;
bottom: 114%;
left: 0;
border-radius: 4px;
box-shadow: 0px 0px 6px 0px rgba(0, 0, 0, 0.3);
padding: 8px 0;
background-color: ${p => p.theme.color.contextMenuBackground};
color: ${p => p.theme.color.contextMenuForeground};
`

const MenuItem = styled('li')`
list-style-type: none;
padding: 10px 18px;
outline: none;
font-size: 12px;

&:hover,
&:focus {
background-color: ${p => p.theme.color.contextMenuHoverBackground};
color: ${p => p.theme.color.contextMenuHoverForeground};
}

&:active {
// TODO(petemill): Theme doesn't have a context menu interactive color,
// make one and don't make entire element opaque.
opacity: .8;
}

&:focus-visible {
outline: solid 1px ${p => p.theme.color.brandBrave};
}
`

export default function PublisherMetaComponent (props: Props) {

const [isMenuOpen, setIsMenuOpen] = React.useState(false)

const triggerElementRef = React.useRef<HTMLElement>(null)

const onClickCloseMenu = React.useCallback((e: MouseEvent) => {
const triggerElement = triggerElementRef.current
if (!triggerElement || triggerElement.contains(e.target as Node)) {
return
}
setIsMenuOpen(false)
}, [setIsMenuOpen])

const onKeyDown = React.useCallback((e: KeyboardEvent) => {
if (e.defaultPrevented) {
return
}
if (e.key === 'Escape') {
setIsMenuOpen(false)
}
}, [setIsMenuOpen])

const toggleMenu = React.useCallback((e: React.MouseEvent) => {
e.stopPropagation()
e.preventDefault()
setIsMenuOpen((value) => !value)
}, [setIsMenuOpen])

// Setup or remote event listeners when opens or closes
// or this element is removed.
React.useEffect(() => {
const removeEventListeners = () => {
window.removeEventListener('click', onClickCloseMenu)
window.removeEventListener('keydown', onKeyDown)
}
if (!isMenuOpen) {
removeEventListeners()
} else {
// TODO(petemill): set element focus when using keyboard arrows.
window.addEventListener('click', onClickCloseMenu)
window.addEventListener('keydown', onKeyDown)
}
return removeEventListeners
}, [isMenuOpen])

const onClickDisablePublisher = React.useCallback(() => {
props.onSetPublisherPref(
props.publisher.publisher_id,
false
)
}, [props.publisher, props.onSetPublisherPref])

const commandText = React.useMemo<string>(() => {
const raw = getLocale('braveTodayDisableSourceCommand')
const publisherIndex = raw.indexOf('$1')
if (publisherIndex === -1) {
console.warn('Locale string for braveTodayDisableSourceCommand did not have a $1 replacement area for publisher name.', raw)
return `${raw} ${props.publisher.publisher_name}`
}
return raw.substr(0, publisherIndex) +
props.publisher.publisher_name +
raw.substr(publisherIndex + 2)
}, [props.publisher.publisher_name])

return (
<Component>
{props.publisher.publisher_name}
</Component>
<PublisherMeta>
<Trigger
className={isMenuOpen ? isOpenClassName : undefined}
onClick={toggleMenu}
innerRef={triggerElementRef}
aria-haspopup='true'
aria-expanded={isMenuOpen ? 'true' : 'false'}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I might be wrong but won’t this translate appropriately if you just set the property to the variable? Ex it handles bool to string conversion

>
<Text>
{props.publisher.publisher_name}
</Text>
{isMenuOpen &&
<Menu
role='menu'
>
<MenuItem
role='menuitem'
tabIndex={0}
onClick={onClickDisablePublisher}
>
{commandText}
</MenuItem>
</Menu>
}
</Trigger>
</PublisherMeta>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@ import CardImage from '../CardImage'
import PublisherMeta from '../PublisherMeta'
import useScrollIntoView from '../../useScrollIntoView'
import useReadArticleClickHandler from '../../useReadArticleClickHandler'
import { OnReadFeedItem } from '../../'
import { OnReadFeedItem, OnSetPublisherPref } from '../../'
// TODO(petemill): Large and Medium article should be combined to 1 component.

interface Props {
content: (BraveToday.Article | undefined)[]
publishers: BraveToday.Publishers
articleToScrollTo?: BraveToday.FeedItem
onReadFeedItem: OnReadFeedItem
onSetPublisherPref: OnSetPublisherPref
}

type ArticleProps = {
item: BraveToday.Article
publisher?: BraveToday.Publisher
shouldScrollIntoView?: boolean
onReadFeedItem: OnReadFeedItem
onSetPublisherPref: OnSetPublisherPref
}

const LargeArticle = React.forwardRef<HTMLElement, ArticleProps>(function (props: ArticleProps, ref) {
Expand All @@ -44,7 +46,12 @@ const LargeArticle = React.forwardRef<HTMLElement, ArticleProps>(function (props
<Card.Time>{item.relative_time}</Card.Time>
{
publisher &&
<PublisherMeta publisher={publisher} />
<Card.Publisher>
<PublisherMeta
publisher={publisher}
onSetPublisherPref={props.onSetPublisherPref}
/>
</Card.Publisher>
}
</Card.Content>
</a>
Expand Down Expand Up @@ -78,6 +85,7 @@ const CardSingleArticleLarge = React.forwardRef<HTMLElement, Props>(function (pr
item={item}
shouldScrollIntoView={shouldScrollIntoView}
onReadFeedItem={props.onReadFeedItem}
onSetPublisherPref={props.onSetPublisherPref}
/>
)
})}
Expand Down
Loading