Redesign Stencila theme and add Wilmore #85

alex-ketch · 2020-02-27T03:20:37Z

This PR makes several changes, fixes, and improvements. Please see the atomic commit messages for the changelog.

Highlights:

"Fleshed out" Skeleton theme (a bit ironic, I know 😄)
New Stencila theme
Tweaked, and re-named old Stencila theme to Wilmore
Utility semantic selectors for targeting groups of elements

Closes #70
Closes #73

On another note, there is a pretty severe bug when hot module reloading during development which makes things unworkable. See my comment here, in short on rebuilds, CSS from unrelated files end up in the compiled output. Only on development and only during re-builds, so doesn't affect production releases.

Theres a simple workaround, or you can disablepostcss-combine-media-query during local development.

If the author doesn't get back to us soon, I will fork the repo.

Close #70

Close #73

codecov-io · 2020-02-27T15:54:34Z

Codecov Report

Merging #85 into next will decrease coverage by 3.55%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             next     #85      +/-   ##
=========================================
- Coverage   91.05%   87.5%   -3.56%     
=========================================
  Files           3       4       +1     
  Lines         123     144      +21     
  Branches       35      39       +4     
=========================================
+ Hits          112     126      +14     
- Misses         11      18       +7

Impacted Files	Coverage Δ
src/themes/index.ts	`100% <ø> (ø)`	⬆️
src/util/index.ts	`100% <100%> (+0.99%)`	⬆️
src/extensions/code/index.ts	`61.9% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eddec64...d6ed29e. Read the comment docs.

davidcmoulton · 2020-02-28T00:18:01Z

README.md

+
+| Selector            | Description                                                                                                                                                                              | Target                                                                                                                        |
+| :------------------ | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- |
+| `:--root`           | It maps to the root element generated by Encoda. This is done to avoid potential clashes with external stylesheets, and to ensure that Thema only styles semantically annotated content. | `[data-itemscope=root]`                                                                                                       |


Not sure what hard dependency there might be on the name from Encoda, but this is very similar to the :root CSS selector. Do you think it's perhaps worth renaming to :--Root to make it less similar?

Hmmm, during development I did mistakenly type :root by accident once or twice, so do see the concern.
Not sure if it's an implementation detail or if the end-user of this selector would care, but the naming convention of TitleCase vs camelCase depends on if the selector targets a MicroData itemtype vs itemprop.
Do you think clarifying the fact that this selector is supposed to be used in place of :root, and adding a Stylelint warning would be enough to helping catch issues?

I share the concern about similarity between :--root and :root. On one hand it is good because it signals the intent, but on the other, can cause confusion. Regardless of either, it is error prone.

I think Alex's suggestions are good. Let's try them and see if the issue arises again. It's not a painful change if we do need to rename in the future.

davidcmoulton · 2020-02-28T00:24:00Z

src/selectors.css

+/**
+ * Group element selectors
+ */
+@custom-selector :--ListTypes :--Article:--root > :--affiliations,:--Collection,:--List,:--references > ol;


Do you have a view on whether a it should be possible / allowable for a particular theme to define its own custom selectors, or whether you see this as a core thema fuction and so should only happen here?

I think theme authors should have freedom to define similar selectors on a per theme basis if needed, given that they do so in a way that stays within the boundaries of the project goals (being a good CSS citizen when used in third-party contexts, and avoiding coupling of the selector targets to specific website HTML structure so as to keep the themes useful for multiple authors).

These selectors were the ones that I felt I was reaching for multiple times, and thought they would be useful across themes.

+1 to "theme authors should have freedom to define similar selectors on a per theme basis if needed"

davidcmoulton · 2020-02-28T00:30:21Z

src/themes/wilmore/README.md

+A theme well suited for consuming long-form manuscripts and prose. Named after Edmond Dantés' alias, [“Lord Wilmore: An
+Englishman, and the persona in which Dantès performs random acts of
+generosity.“](https://en.wikipedia.org/wiki/The_Count_of_Monte_Cristo#Edmond_Dantès_and_his_aliases)


Love it! Although if a "Mondego" theme appears here I may start to worry 😉

nokome

Thanks @alex-ketch. I will push a commit that, I think, better co-locates the selector changes for Prism for you to review.

README.md

nokome · 2020-02-28T05:03:40Z

README.md

+
+| Selector            | Description                                                                                                                                                                              | Target                                                                                                                        |
+| :------------------ | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- |
+| `:--root`           | It maps to the root element generated by Encoda. This is done to avoid potential clashes with external stylesheets, and to ensure that Thema only styles semantically annotated content. | `[data-itemscope=root]`                                                                                                       |


I share the concern about similarity between :--root and :root. On one hand it is good because it signals the intent, but on the other, can cause confusion. Regardless of either, it is error prone.

I think Alex's suggestions are good. Let's try them and see if the issue arises again. It's not a painful change if we do need to rename in the future.

src/scripts/selectors.ts

nokome · 2020-02-28T05:24:24Z

src/selectors.css

+/**
+ * Group element selectors
+ */
+@custom-selector :--ListTypes :--Article:--root > :--affiliations,:--Collection,:--List,:--references > ol;


+1 to "theme authors should have freedom to define similar selectors on a per theme basis if needed"

src/themes/skeleton/styles.css

The advantage of putting this inside the `code` extension is that it is co-located with other aspects of integrating with PrismJS and is less invasive into other code. Also adds a test that the DOM mods are doing what is expected.

nokome · 2020-02-28T07:50:13Z

@alex-ketch I've push a commit that refactors where the PrimJS-ized selectors go. That seems to work:

However, I'm not sure whether the DOM manipulations in the code extension are working.

I don't think we should spend too much time on these workarounds for Prism - it may be more productive to create <stencila-code-fragment> and <stencila-code-block> web components instead - and perhaps use it's tokenization but our own themeable CSS for those tokens.

alex-ketch · 2020-02-28T18:15:03Z

I've made some followup changes based on the comments @nokome.
Please take a look and merge if everything looks good.

nokome · 2020-02-28T21:19:05Z

src/extensions/code/styles.css

-@custom-selector :--CodeBlock [itemtype~='http://schema.stenci.la/CodeBlock'], [itemtype~='http://schema.stenci.la/CodeBlock'] > [class*=language-];
+
+/* prettier-ignore */
+@custom-selector :--CodeBlock [itemtype~='http://schema.stenci.la/CodeBlock'], [itemtype~='http://schema.stenci.la/CodeBlock'][class*=language-];


I used [itemtype~='http://schema.stenci.la/CodeBlock'] > [class*=language-] because Encoda puts the class on the <code> element, not it's parent <pre> element. However, it seems that PrismJS decided to 'transfer' the class to the pre. I which case [itemtype~='http://schema.stenci.la/CodeBlock'][class*=language-] is fine and make more sense. Just noting that there is some magic there. All there more reason to move towards Web Components for CodeBlocks and CodeFragments I feel.

stencila-ci · 2020-02-28T21:35:20Z

🎉 This PR is included in version 1.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

alex-ketch added 12 commits February 26, 2020 22:10

fix(HMR): Fix stack overflow due to double HMR instances

190a891

docs(README): Fix formatting and typos

9b03044

feat(Selectors): Generate PrismJS compatible code selectors

5f34f55

fix(Nature): Fix monospace font-family name

97f92ae

feat(Stylelint): Allow targeting Stencila WebComponents

4d4e079

fix(Browser support): Transform CSS4 :not() usage to CSS3 spec

1317b7c

feat(Selectors): Generate utility selectors for selecting element groups

49f3d38

feat(Skeleton): Add basic styling to Skeleton theme

afffdbd

feat(Stencila/Wilmore): Fork and replace existing Stencila theme

5bd5162

Close #70

refactor(*): Use new :--root custom selector

e22a489

Close #73

fix(README): Fix generation of MD table for list of Themes and Extension

5039e41

docs(README): Fix MD syntax & regenerate HTML

b8be23b

alex-ketch requested review from nokome and davidcmoulton February 27, 2020 03:20

This was linked to issues Feb 27, 2020

Repurpose current Stencila theme #70

Closed

Use the itemscope="root" target added by Encoda for storing CSS variables #73

Closed

Fix typo and failing test

4bc3138

alex-ketch force-pushed the stencila-to-wilmore branch from 6f79786 to 4bc3138 Compare February 27, 2020 15:39

davidcmoulton reviewed Feb 28, 2020

View reviewed changes

nokome suggested changes Feb 28, 2020

View reviewed changes

nokome reviewed Feb 28, 2020

View reviewed changes

src/themes/skeleton/styles.css Outdated Show resolved Hide resolved

refactor(Prism selectors): Define within code extension

5648f0b

The advantage of putting this inside the `code` extension is that it is co-located with other aspects of integrating with PrismJS and is less invasive into other code. Also adds a test that the DOM mods are doing what is expected.

alex-ketch added 2 commits February 28, 2020 12:05

refactor(Skeleton): Remove redundant selector, and fix formatting

d6ed29e

docs(Skeleton): Fix formatting in README

42f582c

nokome approved these changes Feb 28, 2020

View reviewed changes

nokome merged commit 93c07a2 into next Feb 28, 2020

stencila-ci added the released on @next label Feb 28, 2020

nokome deleted the stencila-to-wilmore branch February 29, 2020 01:26

alex-ketch mentioned this pull request Mar 10, 2020

Add option to only run rules for new cards alex-page/github-project-automation-plus#27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redesign Stencila theme and add Wilmore #85

Redesign Stencila theme and add Wilmore #85

alex-ketch commented Feb 27, 2020 •

edited

Loading

codecov-io commented Feb 27, 2020 •

edited

Loading

davidcmoulton Feb 28, 2020

alex-ketch Feb 28, 2020

nokome Feb 28, 2020

davidcmoulton Feb 28, 2020

alex-ketch Feb 28, 2020

nokome Feb 28, 2020

davidcmoulton Feb 28, 2020

nokome left a comment

nokome Feb 28, 2020

nokome Feb 28, 2020

nokome commented Feb 28, 2020

alex-ketch commented Feb 28, 2020

nokome Feb 28, 2020

stencila-ci commented Feb 28, 2020

Redesign Stencila theme and add Wilmore #85

Redesign Stencila theme and add Wilmore #85

Conversation

alex-ketch commented Feb 27, 2020 • edited Loading

codecov-io commented Feb 27, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nokome left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nokome commented Feb 28, 2020

alex-ketch commented Feb 28, 2020

Choose a reason for hiding this comment

stencila-ci commented Feb 28, 2020

alex-ketch commented Feb 27, 2020 •

edited

Loading

codecov-io commented Feb 27, 2020 •

edited

Loading