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

Add option to inject custom CSS #1193

Merged
merged 11 commits into from
Mar 23, 2018
Merged

Add option to inject custom CSS #1193

merged 11 commits into from
Mar 23, 2018

Conversation

ramlmn
Copy link

@ramlmn ramlmn commented Mar 20, 2018

Still a WIP, need to change some wording and layout.

Most important: need some opinions!

Closes #1186.

options after

<textarea name="disabledFeatures" rows="3" placeholder="For example: &#10;mark-unread hide-own-stars"></textarea>
<div class="small">
<strong>Notice:</strong> Mostly untested; only JavaScript-based features can be disabled.
</div>
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem necessary. Also p>div isn't valid HTML

Copy link
Author

Choose a reason for hiding this comment

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

Having class .small for p is of no use as textarea gets an exclusive font-size of 12px, but mostly a WIP.

</p>
<p>
<label>
<input type="checkbox" name="logging">
Enable logging
<span>Enable logging</span>
Copy link
Member

Choose a reason for hiding this comment

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

Nor does this. If you want to add spacing you can add margin-right to input[type='checkbox']

label {
display: flex;
align-items: center;
}
Copy link
Member

Choose a reason for hiding this comment

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

⬆️❓

Copy link
Author

Choose a reason for hiding this comment

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

Before After
image image

Just some vertical alignment!

Copy link
Member

Choose a reason for hiding this comment

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

vertical-alignment: ---px on the checkbox

}
.small {
font-size: 0.9em;
}

p {
margin: 0 0 1em;
}
Copy link
Member

Choose a reason for hiding this comment

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

margin-bottom already seems to be 1em and I think margin-top looks better at 1em (default)

Copy link
Author

Choose a reason for hiding this comment

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

As you know vertical margins do collapse, and margin-top of 0 makes h2 elements and p to be closer, making more content fit in view.

Copy link
Member

Choose a reason for hiding this comment

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

Then we only need margin-top: 0

@@ -10,7 +10,7 @@ <h2>Disable features</h2>
List the features to disable, by <a href="https://github.com/sindresorhus/refined-github/tree/master/source/features" target="_blank">filename</a>. Enable logging to show in the console what features are enabled on each page.
</p>
<p class="small">
<textarea name="disabledFeatures" rows="3" placeholder="For example: &#10;mark-unread hide-own-stars"></textarea> <br>
<textarea name="disabledFeatures" rows="3" placeholder="For example: &#10;mark-unread hide-own-stars"></textarea>
Copy link
Member

Choose a reason for hiding this comment

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

👍


export const injectCustomCSS = async () => {
const {customCSS = ''} = await options;

Copy link
Member

Choose a reason for hiding this comment

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

if customCSS === '', return

resize: vertical;
}
label {
vertical-align: -0.16em;
Copy link
Author

Choose a reason for hiding this comment

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

@bfred-it Can you verify this value of -0.16em? Gives me 2px for 12px font size (2 / 12 ~= 0.16).

@fregante
Copy link
Member

Can you post a screenshot so others can see without checking out?

@hkdobrev
Copy link
Contributor

I'm reposting @ramlmn's screenshot from the issue:

image

@ramlmn
Copy link
Author

ramlmn commented Mar 21, 2018

Thanks @hkdobrev!

@fregante fregante merged commit fcab0b3 into refined-github:master Mar 23, 2018
@fregante
Copy link
Member

Thanks @ramlmn! :)

@fregante fregante mentioned this pull request Mar 23, 2018
@ramlmn ramlmn deleted the add-custom-css-option branch March 24, 2018 09:17
@sindresorhus
Copy link
Member

I've noticed that if I write some CSS in the textarea and then reload the extension on the Extensions page, the CSS is gone.

@fregante
Copy link
Member

Look in the console, you should see the logging from webext-options-sync. It worked for me every time I reloaded

@sindresorhus
Copy link
Member

@bfred-it I write the CSS, close options, click reload, and this is logged:

background.js:212 Appling definitions
background.js:213 Current options: {disabledFeatures: "", logging: false, customCSS: "foo"}
background.js:215 Running 2 migrations

Then I click the reload button again and this is logged:

Appling definitions
background.js:213 Current options: {disabledFeatures: "", logging: false}
background.js:215 Running 2 migrations

@fregante
Copy link
Member

fregante commented Mar 24, 2018

Oh, darn, true. It’s missing a default in background.js so the migration drops it. Edit: fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants