Use scss by jkowalleck · Pull Request #385 · pytest-dev/pytest-html

jkowalleck · 2020-11-27T13:50:09Z

as in #383 (comment)
this very PR causes the style.css to be generated via SCSS.

this PR in on top of #384.
✔️ as #384 was merged into master, this very PR was rebased.onto upstram master

this was a draft - need to clarify how far the changes of this PR might go.
@ssbarnea, do you think i should introduce a full-featured use of sass-variables and -arithmetic, to calculate some of the used values dynamically? Or should this be an extra PR (which i prefer)?

changes in this PR:

added editorconfig, linter & compiler for sass/scss
moved src/pytest_html/resources/style.css to src/layout/css/style.scss - untouched, no modifications to the file.
compiled src/layout/css/style.scss to src/pytest_html/resources/style.css

ssbarnea · 2020-11-27T15:19:55Z

I think is better to keep the variable introduction separately. I would also mention the use os SCSS happens at compile time.

What I would really want to see, preferably before introducing the dark mode is a sample output file, maybe even as a docs job.

One thing that I found annoying was that is not easy to compare the themes we have. If we would have an example report that we render with each of them, it would make much easier for the user to make a decision around which theme to use, or for us to review changes around them.

I guess its is the time to have our own RTD page. I only hope that RTD allows us to display sample reports inside the docs.

jkowalleck · 2020-11-27T15:44:44Z

@ssbarnea, what do you mean with

I would also mention the use os SCSS happens at compile time.

--

regarding

One thing that I found annoying was that is not easy to compare the themes we have. If we would have an example report that we render with each of them, it would make much easier for the user to make a decision around which theme to use, or for us to review changes around them.

I think alike. it was hard for me to see if my color changes did not break anything.
Would come in handy, if there was a dummy-pytest that could run and generate some test results including all the known features: console output logs for stderr and stdout including all ansi codes, added media for images and videos, test results for pass, warn(skip) and fail ....

--

i think introducing sass now would be okay.
since the source scss is an untouched copy of the original css - and the newly generated css is exactly the same like before but has some white space changes.
for further changes, well ... i would wait until a proper test bed is in place.

gnikonorov

Thanks for this @jkowalleck !

I have a few minor comments and requests to split out things that aren't directly related into separate issues and prs, but I think this is awesome.

.editorconfig

gnikonorov · 2020-11-28T03:37:30Z

.pre-commit-config.yaml

        files: ^(CHANGES.rst|development.rst|README.rst)$
        language: python
        additional_dependencies: [pygments, restructuredtext_lint]
+-   repo: https://github.com/elidupuis/mirrors-sass-lint


Can we use this scss linter instead? It's more actively maintained and from the pre-commit org which makes me feel like it's more likely to stay maintained.

Also, if we move to the above linter, we can specify the revision as a tag instead of a sha. I think it's easier for people to read and understand tags v shas

actually i read about https://github.com/pre-commit/mirrors-scss-lint
this uses https://github.com/sds/scss-lint. and this one states "Consider other tools before adopting SCSS-Lint" in its readme.
so i think this one is worth a discussion, @gnikonorov

What do @ssbarnea and @BeyondEvil think? My main concern is the fact that https://github.com/elidupuis/mirrors-sass-lint hasn't been updated since 2017.

Some projects reach a high level of maturity and won't see any updates for quite some time, that doesn't mean it's not a valuable project.

How about using stylint?

As long as styling is well maintained I have no objections. What do you think @jkowalleck ?

not sure.
need to research if stylint works well for the case and how it needs to be configured.

will come back to this topic later.

@jkowalleck let's not bloat this PR. Can you create a follow up issue for this so we don't lose track of it? I think the current linter is fine for now

created #391 to keep track of it.

src/.gitattributes

gnikonorov · 2020-11-28T03:42:16Z

package.json

+  },
  "scripts": {
-    "test": "grunt test"
+    "test": "grunt test",


Is there anyway we can add a github action step that ensures that scss was compiled into css for all PRs? That way we don't have to accidentally worry about people forgetting to run this before they open a PR.

@ssbarnea what do you think?

very good idea, @gnikonorov.
is this what you meant by "I would also mention the use os SCSS happens at compile time.", @ssbarnea ?

but we might need reproducible builds, then. which means npm's package-lock.json should be under version control.

Let's not bloat this PR. Can you create a follow up issue for this @jkowalleck ?

added #392 to keep track of this topic.

ssbarnea · 2020-11-30T15:45:50Z

@gnikonorov I am ok, up to you to merge. I do not care much about the unmaintained linter, we can swap it when it breaks.

gnikonorov

Last request for changes @jkowalleck.

Can you create follow up issues for the two outstanding comments I had and also add documentation on the development workflow for scss to the development guide?

jkowalleck · 2020-11-30T18:27:45Z

Hello @gnikonorov

Can you create follow up issues for the two outstanding comments I had and also add documentation on the development workflow for scss to the development guide?

issues were created and are linked in the conversation. See SASS/SCSS/CSS linter #391 and github action step that ensures that scss was compiled into css for all PRs #392.
added developer notes for "SASS/SCSS/CSS".

gnikonorov

Thanks for this @jkowalleck !

* use scss to generate `style.css` * added linter for sass/scss * generated `style.css` from scss. * developer notes for SASS/SCSS/CSS Co-authored-by: Gleb Nikonorov <gleb.i.nikonorov@gmail.com>

jkowalleck mentioned this pull request Nov 27, 2020

[PREVIEW] Dark mode and scss #383

Closed

use scss to generate style.css

db0e336

ssbarnea self-requested a review November 27, 2020 15:20

jkowalleck mentioned this pull request Nov 27, 2020

move module to src directory #384

Merged

jkowalleck marked this pull request as ready for review November 27, 2020 17:22

gnikonorov requested changes Nov 28, 2020

View reviewed changes

jkowalleck added 2 commits November 28, 2020 14:41

added linter for sass/scss

d14aee1

generated style.css from scss.

281360f

ssbarnea previously approved these changes Nov 30, 2020

View reviewed changes

gnikonorov requested changes Nov 30, 2020

View reviewed changes

jkowalleck mentioned this pull request Nov 30, 2020

SASS/SCSS/CSS linter #391

Open

developer notes for SASS/SCSS/CSS

681c541

jkowalleck dismissed ssbarnea’s stale review via 681c541 November 30, 2020 18:25

jkowalleck requested review from BeyondEvil, gnikonorov and ssbarnea November 30, 2020 18:29

Merge branch 'master' into use_scss

c2f591b

gnikonorov assigned jkowalleck Dec 1, 2020

gnikonorov approved these changes Dec 1, 2020

View reviewed changes

gnikonorov merged commit 0e82906 into pytest-dev:master Dec 1, 2020

jkowalleck mentioned this pull request Dec 1, 2020

github action step that ensures that scss was compiled into css for all PRs #392

Open

Conversation

jkowalleck commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ssbarnea commented Nov 27, 2020

Uh oh!

jkowalleck commented Nov 27, 2020

Uh oh!

gnikonorov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck Nov 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssbarnea commented Nov 30, 2020

Uh oh!

gnikonorov left a comment

Choose a reason for hiding this comment

Uh oh!

jkowalleck commented Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnikonorov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jkowalleck commented Nov 27, 2020 •

edited

Loading

jkowalleck Nov 28, 2020 •

edited

Loading

jkowalleck commented Nov 30, 2020 •

edited

Loading