Per-origin noScript #21
Conversation
yrliou
force-pushed
the
per_origin_no_script
branch
2 times, most recently
from
41bc7d8
to
14fee84
Compare
yrliou
force-pushed
the
per_origin_no_script
branch
from
233489f
to
00bdf26
Compare
app/components/noScript/noScript.tsx
Outdated
| newAllowOrigins = [...this.state.allowOrigins, origin] | ||
| } | ||
|
|
||
| this.setState({ allowOrigins : newAllowOrigins }) |
There was a problem hiding this comment.
We should dispatch something here that is handled in a reducer like onChangedOrigin(origin).
app/components/noScript/noScript.tsx
Outdated
| } | ||
|
|
||
| render () { | ||
| const originSwitches = this.props.blockedOrigins.map((origin) => ( |
There was a problem hiding this comment.
We should display both allowed origins and blocked origins. I noticed on brianbondy.com after I allow brianbondy.com it no longer shows in the list on the next reload.
Example after allowing brianbondy.com, it leaves with options not so great for the next load. Also doesn't give me a full picture of what will happen on the next load.
app/components/noScript/noScript.tsx
Outdated
| </Column> | ||
| {originSwitches} | ||
| <Column align='flex-end'> | ||
| <BrowserButton id='apply' size='10px' onClick={this.onClick}> {getMessage('noScriptApplyOnce')} </BrowserButton> |
There was a problem hiding this comment.
consider splitting out padding and size and other formatting only to not be inline
app/_locales/am/messages.json
Outdated
| "shieldsHeaderForSite": { | ||
| "message": "Site shield settings for", | ||
| "description": "Label for shields header near current site name" | ||
| }, | ||
| "noScriptApplyOnce": { | ||
| "message": "Apply Once", |
There was a problem hiding this comment.
We could perhaps just call this Apply since there's only 1 option we're prioritizing for now. You can keep internal naming as is for now.
yrliou
force-pushed
the
per_origin_no_script
branch
3 times, most recently
from
de6a9b4
to
ecf932c
Compare
yrliou
force-pushed
the
per_origin_no_script
branch
from
ecf932c
to
b03ff4b
Compare
|
@bbondy PR is updated to address review comments, please take a look again when you're available, thanks! |
app/components/noScript/noScript.tsx
Outdated
| <Column key={origin}> | ||
| <SwitchButton | ||
| id={origin} | ||
| checked={this.props.noScriptInfo[origin].willBlocked} |
Fix brave/brave-browser/issues/6
Add a NoScript component which supports users to allow scripts from a set of origins until navigating away.
See also: brave/brave-core/pull/97