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

Replace primer.css with webext-base-css #895

Merged
merged 11 commits into from
Apr 19, 2020
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = {
'no-restricted-syntax': 0,
'consistent-return': 0,
'array-callback-return': 0,
'jsx-a11y/label-has-for': 0
},
globals: {
fixture: true,
Expand Down
1 change: 1 addition & 0 deletions assets/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<head>
<meta charset="UTF-8" />
<title></title>
<link rel="stylesheet" href="chrome://global/skin/in-content/common.css">
Copy link
Member

Choose a reason for hiding this comment

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

Do you know if this is supposed to work in MS Edge as well? The readme of webext-base-css mentions just Firefox. I'm asking because it looks like they added their own protocol edge://

Copy link
Collaborator Author

@fregante fregante Apr 17, 2020

Choose a reason for hiding this comment

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

Indeed, this only loads in Firefox. Firefox effectively doesn't support options.chrome_style so thankfully this stylesheet fills in.

I don't know how it looks in Edge, but I assume it just matches Chrome.

</head>
<body>
<div id="app"></div>
Expand Down
38 changes: 27 additions & 11 deletions packages/helper-settings/SettingsForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ export default class Form extends Component {
async validateToken() {
const { githubToken } = this.state;

if (!this.tokenInputEl) {
this.tokenInputEl = document.querySelector('.js-token');
}
this.tokenInputEl.setCustomValidity('');

if (!githubToken) {
this.setState({
errorMessage: undefined,
Expand All @@ -53,8 +58,18 @@ export default class Form extends Component {
if (!response.login) {
this.setState({
tokenLoaded: false,
errorMessage: response.message || 'Something went wrong',
});

let message = 'The token could not be validated';
if (
response.message &&
response.message.toLowerCase() === 'bad credentials'
) {
message = 'Your token is not valid';
}

this.tokenInputEl.setCustomValidity(message);
this.tokenInputEl.reportValidity();
Copy link
Member

Choose a reason for hiding this comment

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

This solution is inspirited by https://github.com/developit/preact-mdl/blob/master/src/index.js#L873-L878 so it shouldn't be completely wrong 😉

return;
}

Expand All @@ -68,10 +83,6 @@ export default class Form extends Component {
});
}

tokenMessage() {
return <div className="flash flash-success">Token successfully added</div>;
}

render(props, state) {
const { errorMessage, tokenLoaded } = this.state;

Expand All @@ -80,10 +91,13 @@ export default class Form extends Component {
onChange={this.onBlur.bind(this)}
onSubmit={(event) => event.preventDefault()}
>
{tokenLoaded && this.tokenMessage()}
<div className="flash-success">
{tokenLoaded && '✔ Token successfully added!'}
</div>
<Input
type="password"
name="githubToken"
className="js-token"
label="Access token"
description={githubTokenDescription()}
value={state.githubToken}
Expand All @@ -93,7 +107,7 @@ export default class Form extends Component {
setTimeout(this.validateToken.bind(this), 100);
}}
/>
<p className="note ">
<p>
For public repositories,{' '}
<a
href="https://github.com/settings/tokens/new?scopes=public_repo&description=OctoLinker"
Expand All @@ -120,9 +134,11 @@ export default class Form extends Component {
<strong>repo</strong>
</code>{' '}
permissions. Then copy and paste it into the input field above.
<details className="mt-3">
</p>
<p>
<details>
<summary>Why is a GitHub token needed?</summary>
<p className="note">
<p>
OctoLinker uses the{' '}
<a
href="https://developer.github.com/v3/"
Expand All @@ -135,7 +151,7 @@ export default class Form extends Component {
unauthenticated requests to the GitHub API. However, there are two
situations when requests must be authenticated:
</p>
<p className="note ml-5">
<p>
<ul>
<li>You access a private repository</li>
<li>
Expand All @@ -150,7 +166,7 @@ export default class Form extends Component {
</li>
</ul>
</p>
<p className="note">
<p>
When that happens, OctoLinker needs an GitHub access token in
order to continue to work.
</p>
Expand Down
27 changes: 14 additions & 13 deletions packages/helper-settings/components/checkbox.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { h } from 'preact';

export default ({ name, label, description, checked, onClick }) => (
<div className="form-checkbox">
<label htmlFor={name}>
<input
id={name}
name={name}
type="checkbox"
checked={checked}
onClick={onClick}
/>
{label}
</label>
<p className="note">{description}</p>
</div>
<p className="form-checkbox">
<input
id={name}
name={name}
type="checkbox"
checked={checked}
onClick={onClick}
/>
<span>
<label htmlFor={name}>{label}</label>
<br />
<span className="note">{description}</span>
</span>
</p>
);
4 changes: 1 addition & 3 deletions packages/helper-settings/components/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import 'primer-core/build/build.css';
import 'primer-forms/build/build.css';
import 'primer-product/build/build.css';
import 'webext-base-css';

export { default as Input } from './input';
export { default as Checkbox } from './checkbox';
43 changes: 18 additions & 25 deletions packages/helper-settings/components/input.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,29 @@
/* eslint jsx-a11y/label-has-for: 0 */

import { h } from 'preact';

const validationClassName = (error) => (error ? ' errored' : '');

export default ({
name,
label,
description,
error,
value,
onInput,
className,
type = 'text',
}) => (
<dl className={`form-group${validationClassName(error)}`}>
<dt>
<label htmlFor={name} id={name}>
{label}
</label>
</dt>
<dd>
<input
style={{ width: '100%' }}
className="form-control"
type={type}
id={name}
name={name}
value={value}
onInput={onInput}
/>
</dd>
{error && <dd className="error">{error}</dd>}
<p className="note">{description}</p>
</dl>
<p>
<label htmlFor={name} id={name}>
<strong>{label}</strong>
</label>

<input
style={{ width: '100%' }}
className={className}
type={type}
id={name}
name={name}
value={value}
onInput={onInput}
/>

<div className="note">{description}</div>
</p>
);
4 changes: 1 addition & 3 deletions packages/helper-settings/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
"dependencies": {
"linkstate": "^1.1.0",
"preact": "^10.4.0",
"primer-core": "^7.0.0",
"primer-forms": "^3.0.0",
"primer-product": "^6.0.0",
"webext-base-css": "^1.1.0",
fregante marked this conversation as resolved.
Show resolved Hide resolved
"webextension-polyfill": "^0.6.0"
}
}
12 changes: 1 addition & 11 deletions packages/helper-settings/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,4 @@
import { h, render } from 'preact';
import SettingsForm from './SettingsForm';

const App = () => (
<div className="d-flex flex-justify-center">
<div className="four-fifth column">
<div className="Box-row">
<SettingsForm />
</div>
</div>
</div>
);

render(<App />, document.getElementById('app'));
render(<SettingsForm />, document.getElementById('app'));
35 changes: 30 additions & 5 deletions packages/helper-settings/style.css
Original file line number Diff line number Diff line change
@@ -1,11 +1,36 @@
body {
html {
user-select: none;
}

label {
font-size: 12px;
body, p { /* p required to override chrome_style */
line-height: 1.5;
}

.form-checkbox input[type=checkbox], .form-checkbox input[type=radio] {
margin-top: 2px;
.form-checkbox {
display: flex;
}

code {
font-size: 1rem;
}

.note {
display: block;
opacity: 0.6;
}

input[type='checkbox'] {
margin-inline-end: 0.6em;
margin-top: 0.2em;
}

.flash-success {
height: 0.5rem;
text-align: center;
}

@media (prefers-color-scheme: light) {
.flash-success {
color: #165c26;
}
}
Loading