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

Install styles from *.user.css file #134

Merged
merged 252 commits into from Nov 14, 2017
Merged

Conversation

eight04
Copy link
Collaborator

@eight04 eight04 commented Aug 5, 2017

Partially implements #101.

With this PR we can install an *.user.css file like:

/* ==UserStyle==
@name Twitter Modal Full Width
@namespace eight04.blogspot.com
@version 0.1.0
@var text video-width 'The width of videos' 854px
==/UserStyle== */

@-moz-document domain("twitter.com") {
.PermalinkOverlay {
	overflow-x: hidden;
}

.PermalinkOverlay-modal {
	position: relative;
	top: 0!important;
	left: 0;
	margin: 60px 70px;
	width: auto;
	min-width: 640px;
}

.permalink-container {
	width: 100%;
	float: none;
}

.TweetArrows {
	width: auto;
	right: -80px;
}

.permalink-container .AdaptiveMedia {
	max-width: 100%;
	max-height: none!important;
	width: auto;
}

.permalink-container .AdaptiveMedia-container,
.permalink-container .AdaptiveMedia-container div[class*=Photo],
.permalink-container .AdaptiveMedia-container .AdaptiveMedia-photoContainer {
	display: inline-block;
	vertical-align: top;
	width: auto;
	height: auto;
	position: relative;
	padding: 0!important;
	margin: 0;
}

.permalink-container .AdaptiveMedia img {
	display: block;
	position: relative;
	top: 0!important;
	left: 0!important;
	width: auto!important;
	height: auto!important;
	max-width: 100%;
}

.permalink-container .AdaptiveMedia.is-generic-video {
	width: 100%;
}

.permalink-container .AdaptiveMedia-video {
	width: var(--video-width, 854px);
	max-width: 100%;
}
}

Main objectives:

  • Install styles from .user.css file.
  • While re-installing, check @namespace and @name, then replace the old one.
  • Auto-update.
  • Generate CSS variables from @var and merge into section.code.
  • Ability to edit @var through a config dialog.

Others:

  • Compile each section with preprocessor.
  • Ability to edit userstyle's source with the editor.

Edit:

  • New config icon.
  • Add configure button to browser action popup?
  • Dynamically load preprocessors.
  • Dynamically load content script.
  • Load install-user-css.js in apply.js?
  • Drop Object.entries.

Edit 9/1:

An example of usercss host on gist
https://gist.githubusercontent.com/eight04/1b9edeb170d9f8bbabfb06dc6627f8f7/raw/foo.user.css

  • Single editor... but how?
  • Continuously watching local file (live reload)?

Edit 10/4:

  • A link to the metadata documentation should be included on the edit page.
  • In the "Applies to" section: add and remove buttons.
  • render the author URL in install page.
  • default the linter to "Stylelint", set the "selector-type-no-unknown" to false
  • Close source tab and install page after installed.
  • Switch editor mode while creating a new style if the source code contains metadata block.
  • Strip = and v in @version.
  • Add a feedback link in the config dialog.
  • Add a confirmation when pasting moz-format code to multi-section editor.
  • Editor: ask the user whether to discard the change when the style updated
  • Use an array as options for @var select.
  • Multi-line string syntax in metadata block.

@tophf
Copy link
Member

tophf commented Aug 6, 2017

@schomery, we need to test if adding a content script declaration in manifest.json disables the extension on update: please make a private copy of current Stylus with a dummy name in your google account, install it, update to this PR, and wait for an update (might be useful to click the "Update extensions now" button on chrome://extensions page). In case it gets disabled, apply.js would have to check location.href and load the corresponding scripts by sending a message to the background page that will do chrome.tabs.executeScript. Or webNavigationListener could do that, which is even simpler.

@tophf
Copy link
Member

tophf commented Aug 6, 2017

@eight04, I haven't reviewed the code yet. Some behavioral observations first:

  • It's possible to install an empty style with no name for invalid usercss, which will make other parts of Stylus fail. No installation should take place in such case. A warning should be displayed I guess.

  • It's better to run the installer on document_end so that the page contents is visible

  • It would make sense to open the style in the editor right after successful installation, probably respecting openEditInWindow preference

  • The config icon should be different, obviously. A gear (cog) with a stroke thickness identical to the adjacent icons. However, I'm not sure it should be in the manager: the editor seems a better place

  • The config dialog is displaying a comment now, which seems a bug:

    clip314-fs8

  • buttons: ['confirmClose'] should be buttons: [t('confirmClose')]

  • All the added messages should be translatable

@eight04
Copy link
Collaborator Author

eight04 commented Aug 6, 2017

The config icon should be different, obviously. A gear (cog) with a stroke thickness identical to the adjacent icons. However, I'm not sure it should be in the manager: the editor seems a better place

Currently it is just a placeholder. Where can I find a proper icon?

The config dialog is displaying a comment now, which seems a bug:

I just changed the @var syntax to @var <key> <label> <value> which caused the problem. And I think we need to discuss how to specify a drop down setting (select an item from a list). I have never used this feature on USO before.

Edit:
I've updated the userstyle example.

buttons: ['confirmClose'] should be buttons: [t('confirmClose')]
All the added messages should be translatable

I haven't figured out how to do this.

@schomery
Copy link
Contributor

schomery commented Aug 7, 2017

please make a private copy of current Stylus with a dummy name in your google account, install it, update to this PR

Unfortunately, I cannot publish any more extensions (even private ones). Mine is full! Anybody else has a dev account?

@tophf
Copy link
Member

tophf commented Aug 7, 2017

Let's implement executeScript scenario I suggested then. We'll use it for freestyler.ws site later as well. @eight04, can you do it in this PR?

@eight04
Copy link
Collaborator Author

eight04 commented Aug 7, 2017

I don't understand. Did you mean the new content script (install-user-css.js) won't be injected into the page after the update?

@eight04
Copy link
Collaborator Author

eight04 commented Aug 7, 2017

Use stylus preprocessor:

/* ==UserStyle==
@name Test
@namespace test.com
@version 0.1.0
@preprocessor stylus
@include http://www.example.com/
@var theme-color 'Pick a color' #000
==/UserStyle== */

body {
	color: theme-color;
	background-color: lighten(theme-color, 80%);
}

@tophf
Copy link
Member

tophf commented Aug 8, 2017

Did you mean the new content script (install-user-css.js) won't be injected into the page after the update?

It will be programmatically injected instead of manifest.json.
Something like this in apply.js:

if (location.href.endsWith('.user.css') && window === top) {
  document.addEventListener('DOMContentLoaded', function _() {
    document.removeEventListener('DOMContentLoaded', _);
    if (document.body.textContent.startsWith('/* ==UserStyle==')) {
      chrome.runtime.sendMessage({
        method: 'injectContentScript',
        js: 'install-user-css.js',
      });
    }
  });
}

And in background.js::onRuntimeMessage:

case 'injectContentScript':
  chrome.tabs.executeScript(sender.tab.id, {
    file: request.js,
    allFrames: false,
    frameId: sender.frameId,
  });

Other observations:

  • don't use Object.entries, it's not available in Chrome 49 which is the minimum version we support. Make sure each language feature you use is compatible with v49.
  • Using devtools performance analyzer I see stylus preprocessor dependency takes as much time to load as our background page. This is bad and wrong. Especially if we add more preprocessors in the future. It should be lazy-loaded only if needed e.g. setTimeout(loadStylusPreprocessor) and function loadStylusPreprocessor() { const el = document.createElement('script'); el.src = '/vendor/stylus/stylus.min.js'; document.head.appendChild(src); }. Of course any dependent code should run after that.

@tophf
Copy link
Member

tophf commented Aug 8, 2017

Correction: any preprocessors should only load when they're actually used because the majority of our users don't use any of them.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 9, 2017

We need to refactor edit.js. It is 2.2k lines now.

@tophf
Copy link
Member

tophf commented Aug 9, 2017

Yes we do need to rewrite it, but that's a separate issue.

@tophf
Copy link
Member

tophf commented Aug 9, 2017

  • After successful installation it would make sense to open the editor with the installed style. Maybe in a new tab or maybe replace the current one.

  • When the same version of usercss is opened in a tab, instead of asking to install it, we should display a message that the same version is already installed.

  • Hiding the name in editor looks weird and bad. Simply make the element disabled and show an (i) icon that would explain the specifics of userstyle mode like other such icons do.

  • Showing a disabled [x] Userstyle mode checkbox is also weird. Currently I don't see why this checkbox is needed even for those who might take advantage of usercss. They can simply drag'n'drop the .user.css file into the browser to install it in this mode.

  • Actually, I doubt single-sectioned usercss is a good idea. Why this restriction? Why the @include weirdness? Why not simply prepend a standard multisection userstyle CSS with /* ==UserStyle== header that contains @name, @namespace or @author, @var? @schomery, @narcolepticinsomniac, @Mottie?

@eight04
Copy link
Collaborator Author

eight04 commented Aug 9, 2017

After successful installation it would make sense to open the editor with the installed style. Maybe in a new tab or maybe replace the current one.

Currently it opens the editor on first installation.
https://github.com/openstyles/stylus/pull/134/files#diff-95c56a0aa187134c80490749cdf67326R356

Showing a disabled [x] Userstyle mode checkbox is also weird. Currently I don't see why this checkbox is needed even for those who might take advantage of usercss. They can simply drag'n'drop the .user.css file into the browser to install it in this mode.

I was thinking to let users to create user-css when writing a new style (right now it is broken). By checking the checkbox, it switches the editor mode.
https://github.com/openstyles/stylus/pull/134/files#diff-a7a23b3c266175f44cb7462c941594d1R1421

Actually, I doubt single-sectioned usercss is a good idea. Why this restriction?

It can contain multiple sections, which are separated with metadata block:

/* ==UserStyle==
@name Test
@namespace test.com
@version 0.1.0
@preprocessor stylus
@include http://www.example.com/*
@var theme-color 'Pick a color' #000
==/UserStyle== */

body {
	color: theme-color;
	background-color: lighten(theme-color, 80%);
}

/* ==UserStyle==
@include http://www.example.com/
==/UserStyle== */

body {
	color: theme-color;
	background-color: darken(theme-color, 80%);
}

Why the @include weirdness?

This is borrowed from userscript. It would be easier to implement other rules like @exclude (exclude specified URLs), @contentType (only include pages with specified content-type), etc. in the future.

Why not simply prepend a standard multisection userstyle CSS with /* ==UserStyle== header that contains @name, @namespace or @author, @var?

I didn't think about that. Maybe splitting different sections into different editors makes more sense? To generate a .user.css file, the style author can simply concatenate them. The advantage of using single editor is just to have a consistent experience compared with normal text editors.

@tophf
Copy link
Member

tophf commented Aug 9, 2017

Currently it opens the editor on first installation.

I've used your sample code and although it's installed but nothing is displayed. I see an error in the background page though: Error in event handler for runtime.onMessage: Error: Missing medata @version and an error in the web page console: Error in event handler for (unknown): TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined. No source line is displayed, but the message isn't shown when Stylus is disabled.

By checking the checkbox, it switches the editor mode.

Well, I don't know your vision of the final UI but I don't like current implementation. Is this usercss so important that it should always be there? Personally, I don't need this feature so I'm more worried about reducing the influence on the standard workflow and UI.

It can contain multiple sections, which are separated with metadata block

Well, you basically ditch the goodness of the existing multi-section editor and the existing standard multi-section userstyle CSS format. I don't see it as a good idea.

It would be easier to implement other rules like @exclude

I suggest using magic comments at the start of each section, which will do the same along with reusing current structure and format.

The advantage of using single editor is just to have a consistent experience compared with normal text editors.

Which normal text editors? Our editor is very far from being even compared to normal CSS editors.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 9, 2017

an error in the background page

Installation error is properly handled in 685f360.

Is this usercss so important that it should always be there?

If you were talking about the 'Userstyle mode' checkbox, how about to hide it when editing regular styles? We can even hide the checkbox when creating styles, which simply remove the ability to create usercss from the editor, then users won't notice any difference if they don't try to install/edit an usercss file.

ditch the goodness of the existing standard multi-section userstyle CSS format.

I don't understand. This PR is based on the current format. It converts usercss to style.sections. However, usercss does have additional fields like style.preprocessor so we can run the preprocessor before saving the style.

using magic comments at the start of each section

If we use multi-section editor to edit usercss, we would still ditch the applies-to toolbar since usercss uses @include rule from the magic comment.

Which normal text editors? Our editor is very far from being even compared to normal CSS editors.

I meant text editors like notepad, sublime, etc. They usually work with a single file.

If the editor does matter, we can simply drop the changes to edit.html and edit.js and make usercss uneditable in style manager, since the main objective of this PR is to install usercss from file (and I think we shouldn't add anything more into edit.js).

@narcolepticinsomniac
Copy link
Member

I'm all for being able to install from different formats. I also appreciate the effort going on here. That said, there's some choices that seem odd to me. Like tophf said, this isn't functionality I ever see myself using, so I've just skimmed the comments here. In that respect, I can review from the viewpoint of a typical clueless end-user.

The easy stuff is the UI choices. I agree with tophf that the disabled checkbox is weird and unnecessary. The option for "Userstyle Mode" should be with all the other options below. Not sure "Userstyle mode" is a great name for it. Looks more like "Userscript format" to me, but maybe that's not accurate either.

The "external" icon is being used incorrectly in the manager:

external icon

Any text in the editor before toggling modes back and forth results in a CodeMirror error shown.

The biggest thing I don't get is why this is not a simple import function like "Moz format", which converts to our standard implementation? I'm not a fan of the disjointed formatting.

@tophf
Copy link
Member

tophf commented Aug 10, 2017

By the standard userstyle CSS format I mean the one used in the classic Stylish-Fx, USO, and freestyler:

@-moz-document domain("stackoverflow.com"), url-prefix("https://stackoverflow.com/documentation/") {
body {
    background-color: #D9DEE0;
}
}

@-moz-document domain("gaming.stackexchange.com") {
body {
    background-color: hsl(221, 47%, 33.7%);
}
}

@-moz-document domain("english.stackexchange.com") {
#footer {
    background-image: none;
}
}

My idea was to augment it with magic comments:

/* ==UserStyle==
@name foo
@namespace bar
@var video-width 'The width of videos' 854px
==/UserStyle== */
@-moz-document domain("stackoverflow.com"), url-prefix("https://stackoverflow.com/documentation/") {
/* @exclude regexp("foo") */
body {
    background-color: #D9DEE0;
}
}

@-moz-document domain("gaming.stackexchange.com") {
/* @exclude url-prefix("https://gaming.stackexchange.com/bar") */
body {
    background-color: hsl(221, 47%, 33.7%);
}
}

@-moz-document domain("english.stackexchange.com") {
#footer {
    background-image: none;
}
}

As you can see, there's no @include. As for @exclude, I'm quite sure it's not a good idea because such file won't be compatible with USO and other userstyle sites. Exclusion can be implemented with negative-lookahead regexp so there's no desperate need for this feature at all. If anything, we can add a simple UI to build such regexps from the standard 4 types of URL functions used in @-moz-document. IIRC I even submitted something similar in my PR for the old Stylish (you can find it in that repo).

Such file can be imported using the existing import button, which will additionally process the magic comments and set usercss mode (note, not userstyle mode, because a userstyle format is a classic userstyle used on USO/Stylish-Fx).

I don't know how to organize the editor UI though.

@narcolepticinsomniac
Copy link
Member

Well, I can offer my opinions on the UI, but tbh, I don't really get the objective here. I understand why a legacy style Stylish editor option would be useful. Nostalgia, familiarity, organization, and plenty of available styles in Moz format. Are there styles available in these formats, or are we just implementing compatibility for the sake of being able to write styles in a different format? I don't get it. Admittedly, I'm not trying all that hard to understand, but this seems like a lot of effort for a feature I don't see being utilized.

@Mottie
Copy link
Member

Mottie commented Aug 10, 2017

The "external" icon is being used incorrectly in the manager:

I think the "external" icon shouldn't be shown because clicking it will take the user back to the source page and cause the editor to open because it's in a userstyle format. I haven't had a chance to test this code, but how does it get updated? I don't see a @updateURL or @downloadURL similar to that used in userscripts.

Are there styles available in these formats, or are we just implementing compatibility for the sake of being able to write styles in a different format?

There aren't any styles in this format, but I think there is a lot of potential having a userstyle that can be installed from a source outside of USO; but I do agree with @tophf about keeping the same CSS format.

As for @exclude, I'm quite sure it's not a good idea because such file won't be compatible with USO and other userstyle sites

I think the @exclude was added to fulfill #113, but as discussed in that issue, it can be handled in the settings. So I vote to remove it from here as well.

@tophf
Copy link
Member

tophf commented Aug 10, 2017

I thought it might be useful to mention my idea about making a single code box that handles multiple sections while preserving the ease of visual manipulation. Turns out CodeMirror provides line widgets which we can show for @-moz-document sections like this:

clip317-fs8

The big advantage is that with just one editor all multi-section styles will be handled internally a lot faster for everything related to DOM (displaying, scrolling, initial loading).

To facilitate navigating between sections we could display a persistent shelf on the bottom with numbers 1 2 3 4 5 ... to scroll to that section. A hover tooltip may display a few lines of that section's code. We can even recognize magic comments /* @section-name foo */ and display them after (instead of) the number.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 10, 2017

@narcolepticinsomniac

why this is not a simple import function like "Moz format", which converts to our standard implementation?

It is not just the format. Moz-format userstyle is just a static CSS file but usercss needs to build the code dynamically according to its configuration.

@tophf's solution looks good. I think the syntax is more CSS-ish. I'll take a look at moz-format parser.

@Mottie

how does it get updated?

Usercss has style.updateUrl property as regular userstyles. It points to the location where the style gets installed.

Currently usercss has its own update process without checking MD5. When updating:

  1. Check if the style is usercss. If not, then go with regular update process.
  2. Download the code from updateUrl.
  3. Parse the text, find the verision.
  4. Compare the version, determine whether it should be updated or skip.
  5. Save the style.

There is a potential issue: usercss might be installed from file:// protocol. I'm quite sure auto-update would not work with it.

@tophf

I have no experience working with CodeMirror. I'll revert the changes to edit/ first.

@narcolepticinsomniac
Copy link
Member

I think there is a lot of potential having a userstyle that can be installed from a source outside of USO; but I do agree with @tophf about keeping the same CSS format.

Alright. I just wanted to make sure I wasn't missing something. As long as we're all on the same page that this is mostly about "future potential", as opposed to being immediately useful, I think it's great. Usually when people show up and randomly start putting in this much effort, it's to fix or improve something that's bugging them personally, not to implement what is currently a fairly abstract concept.

It is not just the format. Moz-format userstyle is just a static CSS file but usercss needs to build the code dynamically according to its configuration.

Still don't understand why it wouldn't be "dynamically" converted upon install. All the "userscript-ish" format is either directly applied, or interpreted as, standard naming, applies to rules, update URLs, and stylesheets. The ability to install a different format is great, but why would that format need to be shown in the editor? I think of this as an installable/importable/exportable format, not a "mode" which should be shown in the editor.

I thought it might be useful to mention my idea about making a single code box that handles multiple sections while preserving the ease of visual manipulation.

Best of a bunch of worlds, huh? Performance improvements of single editor, and the convenience of sections with magic comments, seems like a no-brainer. Would we be losing any functionality for individual sections?

controls

No doubt this'd be "non-trivial", as you like to say, but it does seem pretty awesome conceptually. It might be pretty sweet if magic comments could serve a dual purpose of naming and re-ordering sections.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 11, 2017

It seems that we can't reuse the moz-parser from edit.js. It can only handle regular CSS format and failed with nested rules which commonly used by LESS/SCSS. Maybe I can write a parser based on brace matching, but I doubt it would be compatible with all preprocessors we'd like to support.

@tophf
Copy link
Member

tophf commented Aug 12, 2017

Yeah, I suppose you'll have to include less.min.js in the extension and use it to process the code.

@tophf
Copy link
Member

tophf commented Aug 12, 2017

Alternatively, it's possible to tweak csslint to recognize nested rules. We already patched it locally several times because this library is effectively dead.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 16, 2017

I've finished the moz-format usercss:

/* ==UserStyle==
@name foo
@namespace bar
@version 0.1.0
@var my-color 'Choose a color' #D9DEE0
@preprocessor stylus
==/UserStyle== */

@-moz-document domain("stackoverflow.com"), url-prefix("https://stackoverflow.com/documentation/") {
body {
    background-color: my-color;
    a {
        color: lighten(my-color, 50%);
    }
}
}

@-moz-document domain("gaming.stackexchange.com") {
body {
    background-color: darken(my-color, 50%);
}
}

@-moz-document domain("english.stackexchange.com") {
#footer {
    background-image: none;
}
}

Any advice about the configure icon? Should I draw it myself?

@Mottie
Copy link
Member

Mottie commented Aug 16, 2017

I think the editor should contain the unaltered style... the processed and raw css should be saved separately and the processing should only occur when the changes are saved. All the benefits of using stylus are lost if we present the user with processed css.

Additionally, we should be able to "import" (under Mozilla format) the user.css file.

@eight04
Copy link
Collaborator Author

eight04 commented Aug 16, 2017

I agree. There was a single editor to edit usercss source before the revert (b4b2f61). Currently edit.js is too hard to work with and I think we should split it into different modules. I'm not sure if it should be done in this PR, and there will be a tone of conflicts between #150.

IMHO, we need a PR to refactor edit.js before we can implement the single code block mentioned at #134 (comment)

Also moz-format parser is already decoupled from edit.js: d49f65b

@Mottie
Copy link
Member

Mottie commented Aug 16, 2017

Why not split out the code you are using now, like you did with moz-parser.js? Then it will make the refactoring of edit.js that much easier. My PR is also modifying the edit.js, so I plan to split out the linter functions into a separate file as well.

@eight04
Copy link
Collaborator Author

eight04 commented Nov 9, 2017

Done.
image

@tophf
Copy link
Member

tophf commented Nov 9, 2017

I think it's okay to merge.
@narcolepticinsomniac, @Mottie, one last test please, at least during one day.

@tophf
Copy link
Member

tophf commented Nov 9, 2017

FWIW, zip of locally merged branches: stylus-master&usercss.zip

@Mottie
Copy link
Member

Mottie commented Nov 9, 2017

I'm out of town right now with only my phone until next week

@narcolepticinsomniac
Copy link
Member

I haven't been following all that closely, so with me testing, you're basically getting feedback from a typical clueless end-user.

I really expected configuration options to popup automatically on the installation page. When that didn't happen, I expected them to pop up in the editor page that's opened upon installation. When that didn't happen, I looked around for an option to open the config settings manually and saw none. Did I miss something? If I wasn't aware that they were there somewhere, looking for them in the manager wouldn't be all that obvious. It's fine for them to be there as well, but that shouldn't be the only way to access them. They should open automatically, either before or directly after installation IMO.

When I finally got to it, the config UI is really nice, and everything seems to work well. The message box closing upon save is kinda annoying, since I'm tweaking multiple settings and checking how they look, I have to reopen after every tweak. The example script should be altered to demonstrate font color correctly because changing it for 'body' does nothing.

A few little UI quirks:

Install page header should be on top, and I'd also suggest that the update option could be a simple "Check install URL for updates" or even just "Check for updates" without displaying the whole URL in the UI, which is a little messy looking. Also, blue links don't follow the monochrome protocol.

header

Install message box header/logo is out of whack.

logo

Config message select should resize dynamically:

select

Should numerical inputs use input[type="number"]? I realize this may get pretty complicated because of all the measurement units, but I wonder if filling in an actual field themselves is intimidating and confusing to some users.

input

Another thought I had that I figured I'd throw out there, is that it'd be nice to be able to view the computed code. Not sure how important that is, but I wanted to see it.

@eight04
Copy link
Collaborator Author

eight04 commented Nov 11, 2017

The message box closing upon save is kinda annoying, since I'm tweaking multiple settings and checking how they look, I have to reopen after every tweak.

I had planned to add "configure" action in the popup panel:
image
When the button is clicked, the config dialog shows up so users can change the setting without leaving the site that the style applies to.

I think we can do this in another PR, also to keep the dialog after saving.

I wonder if filling in an actual field themselves is intimidating and confusing to some users.

I guess it should be fine. Userstyles.org doesn't provide numeric field either.

it'd be nice to be able to view the computed code.

Since the style is injected inline, you can find it from the inspector:
image

Changes:

  • Install page
    image
  • <select> can stretch up to 124px (same as the color input):
    image
    image

@narcolepticinsomniac
Copy link
Member

Since the style is injected inline, you can find it from the inspector

I'm aware. I meant a convenient way to view it in the editor. It kinda bugs me that I can't easily view the interpreted code, but maybe that's just me. Not terrible... just feels strange. If nobody agrees, NBD.

<select> can stretch up to 124px (same as the color input)

Difference is, the color input is a known entity. AFAICT, select text can be unpredictable, so if we do set a max-width, it should be at the aesthetically ridiculous point, and even then we might as well throw an ellipsis on it. Even your last photo was still cutting off text:
select

I guess it should be fine. Userstyles.org doesn't provide numeric field either.

NBD either, if no one else feels like it's warranted.

I had planned to add "configure" action in the popup panel

That's cool too. The more ways to access it, the better, but the biggest point I was trying to emphasize is that the config needs to be integrated into the install process. It should pop up automatically, either before or directly after installation. Burying all your hard work elsewhere, where it might go overlooked, makes zero sense. If a style has config options, users should be aware right off the bat, and not have to go looking to click an icon in a completely separate area of the UI.

Other than that, this format is really interesting. If options were more prominent during installation, it'd be a big improvement for users installing, compared to US.o, where advanced settings are buried and surely go overlooked fairly often. Also easier to host pretty much anywhere. The only variable is whether authors will get on board, since they'll have learn the basics. I hope it does catch on, because you've done a nice job with it here.

@tophf tophf merged commit 1d463d7 into openstyles:master Nov 14, 2017
@tophf
Copy link
Member

tophf commented Nov 15, 2017

@eight04, are you interested in speeding up createWidgets function using the approach in manage.js::createStyleElement? Basically, you initialize a dummy element once and set variables that point to various sub-elements, then for each actual element you set the variables, and return a deep-clone of the dummy.

@Mottie
Copy link
Member

Mottie commented Nov 18, 2017

I'm back!

Glad to see this one merged!

@narcolepticinsomniac Did you create a GitHub Dark usercss? I have that on my to-do list, but it'd be great if you already have it done 😜

@narcolepticinsomniac
Copy link
Member

Nah man, wasn't me. My pic is just a screenshot of an @eight04 screenshot with an arrow added to point out an overflow issue. Apparently he has converted it, at least to some degree, to use for testing though.

@eight04
Copy link
Collaborator Author

eight04 commented Nov 19, 2017

@Mottie That usercss is generated by xStyle, by installing userstyle from USO and exporting it.

@Mottie
Copy link
Member

Mottie commented Nov 22, 2017

@eight04 Is there any way I could get you to email me (gmail: wowmotty) the usercss? It'll save me some work since I can't seem to get the GitHub-Dark page on freestyler.ws to even load - look at the author page at the first entry. It doesn't even show the style name, and clicking on the link returns a server error. I reported the problem but haven't gotten a response.

@eight04
Copy link
Collaborator Author

eight04 commented Nov 22, 2017

It was installed from here:
https://userstyles.org/styles/37035/github-dark

Anyway, here is the gist:
https://gist.github.com/eight04/87fde3eeef146c5370d903345da3589d

@Mottie
Copy link
Member

Mottie commented Jul 18, 2018

Hey @eight04! We need to provide AMO with a source for the semver bundle. Would you please add a tag to your build? It might be easier to slap a -beta on the end. I know that isn't a valid semver order to go from v5.4.1 to v5.4.1-beta, but I did something similar for the lz-string-unsafe repo.

@eight04
Copy link
Collaborator Author

eight04 commented Jul 19, 2018

Would it be better if we don't fork the entire node-semver but create a new repo to build it? Then we can use our own tag.

@Mottie
Copy link
Member

Mottie commented Jul 19, 2018

Whichever is easier... I created a update-libraries branch that will update the files using npm and copy over the essential files - https://github.com/openstyles/stylus/blob/update-libraries/tools/update-libraries.js - eventually when we have all the libraries updatable, we'll merge it.

@eight04
Copy link
Collaborator Author

eight04 commented Jul 19, 2018

I made a new repo with latest rollup and semver. The bundle size dramatically decreased (10.3 KB -> 4.67 KB).

I'll archive eight04/node-semver-bundle later.

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

Successfully merging this pull request may close these issues.

None yet

5 participants