Skip to content

Commit

Permalink
ship review tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
Shane Osbourne committed Oct 9, 2023
1 parent 5d3d2ed commit 0714ebe
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 38 deletions.
3 changes: 3 additions & 0 deletions build/app/public/css/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,9 @@ a.link-action--text-micro {
display: block;
padding: 8px 0;
}
a.link-action--rounded {
border-radius: 12px;
}

/**
* DDG Extension Helper Classes
Expand Down
13 changes: 7 additions & 6 deletions build/app/public/css/popup.css
Original file line number Diff line number Diff line change
Expand Up @@ -3140,19 +3140,20 @@ body.environment--macos, body.environment--browser, body.environment--windows, b
padding: 12px 16px;
}
.body--theme-dark .note {
background: rgba(255, 214, 92, 0.2);
background: rgba(255, 222, 122, 0.18);
}

.note--nested {
border-bottom-left-radius: 0;
border-bottom-right-radius: 0;
}

.note--nested-alt {
border-top-left-radius: 0;
border-top-right-radius: 0;
}

.note--nested--alt {
background: #ccdaff;
}
.body--theme-dark .note--nested--alt {
background: rgba(204, 218, 255, 0.18);
}

.note--key-insight {
margin-top: 12px;
Expand Down
15 changes: 9 additions & 6 deletions build/app/public/js/base.js

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions integration-tests/DashboardPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class DashboardPage {
}

async showsToggleFeedbackPrompt() {
await this.page.getByRole('link', { name: 'Report broken site' }).waitFor({ timeout: 1000 })
await this.page.getByRole('link', { name: 'Report broken site.' }).waitFor({ timeout: 1000 })
}

async screenshot(name) {
Expand Down Expand Up @@ -142,7 +142,7 @@ export class DashboardPage {

static async android(page) {
const dash = new DashboardPage(page, { name: 'android' })
await dash.withMarker();
await dash.withMarker()
await dash.loadPage()
await dash.withMocks()
await page.waitForFunction(() => typeof window.__playwright !== 'undefined')
Expand All @@ -151,7 +151,7 @@ export class DashboardPage {

static async windows(page) {
const dash = new DashboardPage(page, { name: 'windows' })
await dash.withMarker();
await dash.withMarker()
await dash.withMocks()
await dash.loadPage()
await page.waitForFunction(() => typeof window.__playwright !== 'undefined')
Expand All @@ -160,7 +160,7 @@ export class DashboardPage {

static async macos(page) {
const dash = new DashboardPage(page, { name: 'macos' })
await dash.withMarker();
await dash.withMarker()
await dash.loadPage()
await dash.withMocks()
await page.waitForFunction(() => typeof window.__playwright !== 'undefined')
Expand All @@ -173,7 +173,7 @@ export class DashboardPage {
*/
static async browser(page) {
const dash = new DashboardPage(page, { name: 'browser' })
await dash.withMarker();
await dash.withMarker()
await dash.withMocks()
await dash.loadPage()
await page.waitForFunction(() => typeof window.__playwright !== 'undefined')
Expand All @@ -182,7 +182,7 @@ export class DashboardPage {

static async ios(page) {
const dash = new DashboardPage(page, { name: 'ios' })
await dash.withMarker();
await dash.withMarker()
await dash.loadPage()
await dash.withMocks()
await page.waitForFunction(() => typeof window.__playwright !== 'undefined')
Expand All @@ -191,7 +191,7 @@ export class DashboardPage {

async withMarker() {
await this.page.addInitScript(() => {
return window.__ddg_integration_test = true
return (window.__ddg_integration_test = true)
})
}

Expand Down Expand Up @@ -396,11 +396,11 @@ export class DashboardPage {
}

async helpIsShown() {
await this.page.getByText('Turning OFF the protections might help the website.').waitFor({ timeout: 1000 })
await this.page.getByText('Turning OFF the protections might help the site.').waitFor({ timeout: 1000 })
}

async clicksReportBroken() {
await this.page.getByRole('link', { name: 'Report broken site' }).click({ timeout: 1000 })
await this.page.getByRole('link', { name: 'Report broken site.' }).click({ timeout: 1000 })
}

async showRemoteDisabled() {
Expand Down
8 changes: 6 additions & 2 deletions shared/js/ui/components/text-link.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@ import { useRipple } from '../hooks/useRipple'
/**
* @param {object} props
* @param {import('preact').ComponentChildren} [props.children]
* @param {boolean} [props.rounded]
* @param {() => void} props.onClick
*/
export function TextLink(props) {
const { onClick } = props
const { onClick, rounded = false } = props

const ref = useRef(null)

useRipple({ ref })

let classNames = [`link-action`, `link-action--text`]
if (rounded) classNames.push(`link-action--rounded`)

return (
<a href="javascript:void(0)" className="link-action link-action--text" draggable={false} ref={ref} onClick={onClick}>
<a href="javascript:void(0)" className={classNames.join(' ')} draggable={false} ref={ref} onClick={onClick}>
{props.children}
</a>
)
Expand Down
2 changes: 1 addition & 1 deletion shared/js/ui/hooks/useRipple.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function useRipple(params) {
useLayoutEffect(() => {
const $el = ref.current
if (!$el) return
if (!isAndroid()) return;
if (!isAndroid()) return
$el.classList.add('material-design-ripple')
const instance = MDCRipple.attachTo($el)
instance.listen('click', function (e) {
Expand Down
6 changes: 3 additions & 3 deletions shared/js/ui/templates/protection-header.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function ProtectionHeader(props) {
{!props.model.isBroken && <HeaderDefault model={props.model} state={state} />}
</div>
<div className="text--center">
<TextLink onClick={onClickTextLink}>
<TextLink onClick={onClickTextLink} rounded={true}>
{buttonText}
</TextLink>
</div>
Expand All @@ -123,7 +123,7 @@ function HeaderDefault(props) {
<div className="padding-x padding-y--reduced">
<ProtectionToggle model={props.model} />
</div>
{props.state === 'site-not-working' && <div className="note note--nested-alt">{text}</div>}
{props.state === 'site-not-working' && <div className="note note--nested note--nested--alt">{text}</div>}
</>
)
}
Expand All @@ -140,10 +140,10 @@ function HeaderDisabled(props) {
}
return (
<>
<div className="note note--nested">{text}</div>
<div className="padding-x padding-y--reduced">
<ProtectionToggle model={props.model} />
</div>
<div className="note note--nested">{text}</div>
</>
)
}
4 changes: 2 additions & 2 deletions shared/js/ui/templates/site.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ function renderReportButton() {
window.dispatchEvent(new CustomEvent('open-feedback'))
}
let root = html`<div class="text--center border-light--top"></div>`
render(<TextLink onClick={onClickTextLink}>{ns.site('websiteNotWorkingQ.title')}</TextLink>, root);
return root;
render(<TextLink onClick={onClickTextLink}>{ns.site('websiteNotWorkingQ.title')}</TextLink>, root)
return root
}

/**
Expand Down
4 changes: 2 additions & 2 deletions shared/locales/en/site.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@
"note": "button label text when we are trying to encourage a user to toggle protections off"
},
"websiteNotWorkingCta": {
"title": "Report broken site",
"title": "Report broken site.",
"note": "button label text for a trigger that shows our feedback form"
},
"websiteNotWorkingAdvice": {
"title": "Turning OFF the protections might help the website.",
"title": "Turning OFF the protections might help the site.",
"note": "help text for "
},
"websiteNotWorkingPromptFollowUp": {
Expand Down
2 changes: 2 additions & 0 deletions shared/scss/_vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ $color--medium-light-platinum: rgba(0, 0, 0, 0.05);
$color--alert-red: #ee1025;

$color-yellow: #fff0c2;
$color-yellow--dark: rgba(#ffde7a, 18%);
$color-note-blue: #ccdaff;
$color-note-blue--dark: rgba($color-note-blue, 18%);

$color-gray-20: #eeeeee;
$color-gray-90: #222;
Expand Down
3 changes: 3 additions & 0 deletions shared/scss/base/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ a {
display: block;
padding: 8px 0;
}
&.link-action--rounded {
border-radius: 12px;
}
}

/**
Expand Down
14 changes: 7 additions & 7 deletions shared/scss/views/_note.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@
border-radius: 12px;
padding: 12px 16px;

// this will override any other margins and ensure there's enough space
.body--theme-dark & {
background: rgba(255, 214, 92, 0.2);
background: $color-yellow--dark;
}
}

.note--nested {
border-bottom-left-radius: 0;
border-bottom-right-radius: 0;
}

.note--nested-alt {
border-top-left-radius: 0;
border-top-right-radius: 0;
}

.note--nested--alt {
background: $color-note-blue;
.body--theme-dark & {
background: rgba($color-note-blue--dark, 18%);
}
}

// this solves the case where the note is embedded inside the key-insight component
Expand Down

0 comments on commit 0714ebe

Please sign in to comment.