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.

Sign up for GitHub

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

Jump to bottom

plugin refactor #409

Merged
merged 11 commits into from
Feb 5, 2021
Merged

plugin refactor #409

merged 11 commits into from
Feb 5, 2021

Conversation

vincerubinetti
Copy link
Collaborator

@vincerubinetti vincerubinetti commented Feb 2, 2021 •
edited
Loading

Closes #268
Closes #241

  • moves repeated and shared/generic functions to separate "core" plugin
  • reorganizes html.yaml config into first and third party plugins
  • removes functionality to set plugin options from url
  • reformats plugins with Prettier (eg 4 space tabs to 2 space)
  • remove (function () { ... } )() anonymizer wrapper, and just make <script> tag into module to keep scope local and avoid function name conflicts. this reduces the indent of the whole script by one level.
  • moves plugin specific CSS to plugins themselves

Originally I wanted all the CSS in the theme for the situation where we create another theme and need to style the plugins in that new theme too. But 1) the default theme was getting to unruly and monolithic 2) it makes more sense to group the plugin concerns together in one file 3) making a new theme has been pushed to the back burner for a long time and 4) when we do make a new theme, there are modern ways to solve the problem of styling the plugins (like CSS variables).

@AppVeyorBot
Copy link

AppVeyor build 1.0.214 for commit 3b59832 failed.

@AppVeyorBot
Copy link

AppVeyor build 1.0.215 for commit 2e27107 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@vincerubinetti
Copy link
Collaborator Author

@dhimmel why is the svg image in the delete-me.md being changed from master to main (and thus being a broken link).

@AppVeyorBot
Copy link

AppVeyor build 1.0.216 for commit a1a5da9 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@AppVeyorBot
Copy link

AppVeyor build 1.0.217 for commit 6e535ad is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@AppVeyorBot
Copy link

AppVeyor build 1.0.218 for commit 2fb0a10 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@rando2 rando2 mentioned this pull request Feb 3, 2021
Closed
@AppVeyorBot
Copy link

AppVeyor build 1.0.219 for commit 727b8ca is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@dhimmel
Copy link
Member

dhimmel commented Feb 4, 2021

@dhimmel why is the svg image in the delete-me.md being changed from master to main (and thus being a broken link).

Ah that happened in 690aac1 and was likely an accident (I must have thought this was a link to manubot/rootstock and not manubot/resources. Rather than revert that change, I am going to update the default branch for resources. I think GitHub will redirect any old links to master.

After that will review this PR! Looking forward to trying the refactored plugins.

dhimmel
dhimmel reviewed Feb 4, 2021
View reviewed changes
Copy link
Member

@dhimmel dhimmel left a comment

Choose a reason for hiding this comment

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

net minus 700 lines... nice!

Played around with manuscript-1.0.219-727b8ca.html and everything looks good, but that wasn't an exhaustive test since the rootstock manuscript is simple and not all the plugins are enabled. @vincerubinetti could you render a manuscript with all plugins active. Is there a place I can see the scite plugin?

moves repeated and shared/generic functions to separate "core" plugin

👍🏻

reformats plugins with Prettier

👍🏻

moves plugin specific CSS to plugins themselves

we can consider also closing #241 since these files now have mixed JS/CSS?

@vincerubinetti
Copy link
Collaborator Author

@vincerubinetti could you render a manuscript with all plugins active. Is there a place I can see the scite plugin?

Going to try to give you a version of the Covid manuscript with all the plugins enabled.

@AppVeyorBot
Copy link

AppVeyor build 1.0.220 for commit 6201821 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@vincerubinetti
Copy link
Collaborator Author

Sorry I uploaded the wrong file. Here's the correct one:

covid-19-manuscript-all-plugins.zip

You'll have to serve the index file from a server.

Here's a preview of the Scite plugin:

image

@AppVeyorBot
Copy link

AppVeyor build 1.0.221 for commit a1919a5 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@dhimmel
Copy link
Member

dhimmel commented Feb 5, 2021

Going to try to give you a version of the Covid manuscript with all the plugins enabled

That manuscript looks great! The scite plugin is cool. As far as the pre-existing plugins, I didn't notice any behavioral differences. Everything worked in Chrome.

@AppVeyorBot
Copy link

AppVeyor build 1.0.222 for commit c011344 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@AppVeyorBot
Copy link

AppVeyor build 1.0.223 for commit 47fceaf failed.

@AppVeyorBot
Copy link

AppVeyor build 1.0.224 for commit eabe74b is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

dhimmel
dhimmel approved these changes Feb 5, 2021
View reviewed changes
Copy link
Member

@dhimmel dhimmel left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks @vincerubinetti for this big refactor. I'll merge now and we can address any @agitter comments in a follow up.

@dhimmel dhimmel merged commit 87c1195 into manubot:main Feb 5, 2021
@vincerubinetti vincerubinetti deleted the plugin-refactor branch February 5, 2021 17:44
@agitter
Copy link
Member

agitter commented Feb 5, 2021

I'll merge now and we can address any @agitter comments in a follow up.

Good plan. I want to review this but won't get to it immediately.

@agitter
Copy link
Member

agitter commented Feb 13, 2021

I reviewed this now and don't have any additional comments. Thanks for the big updates @vincerubinetti. We've been using this in the COVID manuscript for the past week or so, and I haven't noticed any plugin-related issues.

bluegenes added a commit to bluegenes/2021-paper-sourmash-distance that referenced this pull request May 19, 2021
@bluegenes @vincerubinetti
@nfry321 @lubianat @dhimmel @agitter
Rootstock 2021 05 19 (#2)
318a508 
* Webpage: refactor plugins & add scite plugin

merges manubot/rootstock#409

* moves repeated and shared/generic functions to separate "core" plugin
* reorganizes html.yaml config into first and third party plugins
* removes functionality to set plugin options from url
* reformats plugins with Prettier (eg 4 space tabs to 2 space)
* removes anonymizer wrapper. Just make <script> tag into module to keep scope local
  and avoid function name conflicts. this reduces the indent of the whole script by one level.
* moves plugin specific CSS to plugins themselves
* adds scite plugin (uncomment to enable)

* Update scite plugin

merges manubot/rootstock#415

* setup.bash: interactive script to guide setup

merges manubot/rootstock#417
closes manubot/rootstock#401

* Add "gh repo create" to SETUP.md

merges manubot/rootstock#419
closes manubot/rootstock#418

Co-authored-by: Daniel Himmelstein <daniel.himmelstein@gmail.com>
Co-authored-by: Anthony Gitter <agitter@users.noreply.github.com>

* fix link

Co-authored-by: Vincent Rubinetti <vince.rubinetti@gmail.com>
Co-authored-by: nfry321 <58332182+nfry321@users.noreply.github.com>
Co-authored-by: Tiago Lubiana <tiago.lubiana.alves@usp.br>
Co-authored-by: Daniel Himmelstein <daniel.himmelstein@gmail.com>
Co-authored-by: Anthony Gitter <agitter@users.noreply.github.com>
ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this pull request Aug 6, 2024
@ploegieku
Webpage: refactor plugins & add scite plugin
2536c04 
merges manubot/rootstock#409

* moves repeated and shared/generic functions to separate "core" plugin
* reorganizes html.yaml config into first and third party plugins
* removes functionality to set plugin options from url
* reformats plugins with Prettier (eg 4 space tabs to 2 space)
* removes anonymizer wrapper. Just make <script> tag into module to keep scope local
  and avoid function name conflicts. this reduces the indent of the whole script by one level.
* moves plugin specific CSS to plugins themselves
* adds scite plugin (uncomment to enable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhimmel dhimmel dhimmel approved these changes

@agitter agitter Awaiting requested review from agitter

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tooltips plugin malfunctions when citation hidden in <details> Rename themes back to .css from .html
4 participants
@vincerubinetti @AppVeyorBot @dhimmel @agitter