More css vars

[mmkal]

Making this a draft because it's a bigger change and I want to test it thoroughly before declaring ready.

This moves further towards CSS variables and away from LESS variables. The motivation vs last time in #38 is slightly different - that was a bug fix, but this change allows users of the package to update the values of these variables, with higher specificity than in this repo, to change the look and feel of the crossword. Right now that's possible, but impractical since changing, for example, the grid size at a certain breakpoint, involves debugging all the CSS and finding everywhere that the grid size is used. Now users could just add a class like this:

@media min-width(2000px) {
  #myApp .crosswords-js {
    --grid-cell-size: 5rem;
  }
}

That value would be respected everywhere - and assuming this library continues to use the variables, in all future visual elements too.

[pvspain]

Hi Misha @mmkal,

I can see the value in dynamically setting the grid height and width in response to loading a new puzzle, but I don't envision the use cases for dynamically changing other grid view properties?

There are many less variables declared in the source to simplify changing the look and feel. That was the intention of separating out style/cwdimensions.less and style/cwcolors.less.

Are you proposing the changes so users can change the look and feel without modifying the less source, which would require a fork of this repo?

[mmkal]

Are you proposing the changes so users can change the look and feel without modifying the less source, which would require a fork of this repo?

Yes, exactly.

[pvspain]

Are you proposing the changes so users can change the look and feel without modifying the less source, which would require a fork of this repo?

Yes, exactly.

I see. There is potential for side effects with the gridView structure changing outside the JS context. You'll need sanity checking on the new values - with meaningful error messages! - and consider whether the Views need to be rebuilt inside the Controller logic, as well as the consequent changes that will occur in the DOM as the CSS variables change.

On a side note, to upload your PR mods cleanly (src/crossword-controller.mjs), please add the Git pre-commit hook to your local repo, as described in the Quality Assurance section of the root README.md:

To automate all these checks on each commit to your local git repository, create a pre-commit hook in your repository. From the root directory of your repository:

cat << EOF > .git/hooks/pre-commit
#!/bin/sh
npm run spell && \\
npm run prettier:fix && \\
npm run lint:fix && \\
npm test
EOF
chmod u+x .git/hooks/pre-commit

[mmkal]

I see. There is potential for side effects with the gridView structure changing outside the JS context. You'll need sanity checking on the new values - with meaningful error messages! - and consider whether the Views need to be rebuilt inside the Controller logic, as well as the consequent changes that will occur in the DOM as the CSS variables change.

I don't think I follow the concern here. CSS variables can't change the gridView structure, just how it looks. You can just do things like make the cells bigger or smaller, or change the highlight colour from yellow to blue. About error messages, is the concern that someone does something invalid like --highlight-color: oopsatypo? If so, I don't think anything is needed from this library, beyond Chrome dev tools. In that case the highlight color would just be un-set and chrome would show it as crossed out in dev tools.

Thank you for the pre-commit hook tip! Will create it locally.

[pvspain]

I see. There is potential for side effects with the gridView structure changing outside the JS context. You'll need sanity checking on the new values - with meaningful error messages! - and consider whether the Views need to be rebuilt inside the Controller logic, as well as the consequent changes that will occur in the DOM as the CSS variables change.

I don't think I follow the concern here. CSS variables can't change the gridView structure, just how it looks. You can just do things like make the cells bigger or smaller, or change the highlight colour from yellow to blue. About error messages, is the concern that someone does something invalid like --highlight-color: oopsatypo? If so, I don't think anything is needed from this library, beyond Chrome dev tools. In that case the highlight color would just be un-set and chrome would show it as crossed out in dev tools.

CSS values can change the grid structure - your first PR could enable users to set the height and width of the grid. The JS now rebuilds the gridView in newCrosswordGridView() after the CSS values are applied. In this context, the dimensions are bound to the new model dimensions, so all's good in the hood. I don't want to do this on a case-by-case basis.

My concern is to some degree non-technical, relating to the support and governance of this repository. I've worked in software a long time, and we don't have the resources to support this if it becomes popular. Some people have a tendency to lay blame where the symptoms appear - in our gridView - and look at the most recent changes - their own - as a last resort. If we have a strict and clear interface around this module, which stops people from shooting their toes off, it pays off many times over in support effort debugging their problems. If we don't help them, we encounter reputational damage and the project suffers through no inherent fault.

A relevant example is early versions of HTML. The separation between semantics (HTML) and presentation (CSS) became clear in HTML4 , IIRC. The specification still suffers with legacy styling tags. Also, early browsers were quite lenient with poorly-formed HTML - missing end-tags for example. The ongoing support for legacy features makes coding and tool-building more difficult than it need be.

The XML community got that right by ensuring strict conformance and support for schema validation through XSD and associated tools. It is clearly understood that invalid XML documents will be rejected. The effort in parsable XML stays with the document producers, as it must if a technology is to survive, as XML has. Tooling is much simpler to write because use cases stay close to the happy path.

I have been thinking about integrating JSON schema validation as a feature down the track. One concern is the usefulness of the emitted errors as diagnostics. I think the the diagnostics emitted by our own model parser are quite good.

Thank you for the pre-commit hook tip! Will create it locally.

Good-oh! There is an amendment to the pre-commit hook in PR42. The older version worked when npm was also installed outside nvm. To ensure we're using the nvm-controlled version, the script changes to:

cat << EOF > .git/hooks/pre-commit
#!/bin/sh
# Assuming npm managed via nvm
export PATH=$NVM_BIN:\$PATH
npm run spell && \\
npm run prettier:fix && \\
npm run lint:fix && \\
npm test
EOF
chmod u+x .git/hooks/pre-commit

[mmkal]

My concern is to some degree non-technical, relating to the support and governance of this repository. I've worked in software a long time, and we don't have the resources to support this if it becomes popular. Some people have a tendency to lay blame where the symptoms appear - in our gridView - and look at the most recent changes - their own - as a last resort. If we have a strict and clear interface around this module, which stops people from shooting their toes off, it pays off many times over in support effort debugging their problems. If we don't help them, we encounter reputational damage and the project suffers through no inherent fault.

My technical response: In my opinion, LESS (and similarly SASS) variables only exist because they were invented before CSS variables. They are essentially a hack, they copy-paste values into a CSS file for you. CSS variables are the better solution in virtually every case. I can actually more easily shoot myself in the foot without this change, since I can override a value in one place, but not another (for example, I could develop on one screen width, and have everything line up perfectly, then find that nothing lines up perfectly on narrower screens (this is exactly what happened, in fact)).

My less-technical response: I am one of these users now! I shot my toes off, I figured out why, and I have found the solution. This pull request is it. From tinkering with this repo, I can see that with no additional maintenance overhead here all of my needs can be met (in fact - if you look at the LESS, I have actually reduced the amount of code to maintain, reduced copy-pasting of style classes, as well as decreased dependence on LESS overall).

As for those requirements - I need to do the following things for my crossword app:

1. Align certain visual elements with the crossword - meaning, I need to know the size of each cell. I need this to be the correct size at all screen widths.
2. Change the colours for highlighted and active cells. These colours need to apply to all the different places.
3. Re-use some of the (customised) colours in other parts of my app, ensuring they're consistent.

I have looked at my code, and I've looked at crosswords-js code - and it's clear to me the best place to change it is here. It's worth noting that it's not the only option. I could override almost all of the CSS emitted by crosswords-js, but that would be very hard to maintain.

A relevant example is early versions of HTML. The separation between semantics (HTML) and presentation (CSS) became clear in HTML4 , IIRC. The specification still suffers with legacy styling tags. Also, early browsers were quite lenient with poorly-formed HTML - missing end-tags for example. The ongoing support for legacy features makes coding and tool-building more difficult than it need be.

I don't think I understand the analogy here. I don't think I'm proposing any leniency in particular. I'm just proposing using a CSS feature that's supported by every major browser, and makes styling for users of this library significantly easier.

I think my argument is as simple as: in 2023, nobody should be using LESS variables over CSS variables, really for almost any reason. See css-tricks for a breakdown of why.

we don't have the resources to support this if it becomes popular

I do completely understand the hesitancy to increase the official API surface of a library. Which is partly why I didn't explicitly document the new CSS variables in this PR. In the open source projects I maintain, I see the documentation as the official record of what's supported and what's not. I completely agree that it's out of scope for this library to support the arbitrary styling needs of its every user. But using more idiomatic CSS to empower end users who are prepared to dig for it isn't doing that - it's just good sense.

I hope you're not suggesting deliberately making it harder to use this library just to prevent popularity!

Anyway - I'm a bit worried we're talking at cross-purposes here, because from my point of view, this change is a no-brainer. There are really no advantages of LESS variables that I can see whatsoever. So if you still don't agree, maybe it'd be worth chatting over IM briefly? I'm on discord with the same username, or happy to use another platform if you prefer.

[pvspain]

My technical response: In my opinion, LESS (and similarly SASS) variables only exist because they were invented before CSS variables. They are essentially a hack, they copy-paste values into a CSS file for you. CSS variables are the better solution in virtually every case. I can actually more easily shoot myself in the foot without this change, since I can override a value in one place, but not another (for example, I could develop on one screen width, and have everything line up perfectly, then find that nothing lines up perfectly on narrower screens (this is exactly what happened, in fact)).

Yes, I see you've discovered the responsive rules! (media queries). I haven't dug into your changes as its a work in progress. I appreciate you sharing your progress. I have no insurmountable issue with CSS vs LESS. CSS is still playing catch up with the features of the alternative pre-processors. Mixins are a great feature that is sorely missing in CSS. I'm using them for button styling in dev/index.less to reduce cut-n-paste. Style-wise, however, I find the CSS implementation of variables to be clunky and ugly - but that's what comes from design by committee!

As for those requirements - I need to do the following things for my crossword app:

1. Align certain visual elements with the crossword - meaning, I need to know the size of each cell. I need this to be the correct size at all screen widths.

Sure, I imagine there will be a lot of calc()ing with all the variables to get sizing right for responsive concerns.

A relevant example is early versions of HTML. The separation between semantics (HTML) and presentation (CSS)

I don't think I understand the analogy here. I don't think I'm proposing any leniency in particular. I'm just proposing using a CSS feature that's supported by every major browser, and makes styling for users of this library significantly easier.

I think my argument is as simple as: in 2023, nobody should be using LESS variables over CSS variables, really for almost any reason. See css-tricks for a breakdown of why.

I'm a great believer in standards-based design. I'll have a look at your article; I do find the styling crowd to be a bit too zealous and earnest at times. Once CSS catches up on a few features, I'll stick to it - regardless of how ugly it is on the page! :)

I do completely understand the hesitancy to increase the official API surface of a library. Which is partly why I didn't explicitly document the new CSS variables in this PR. In the open source projects I maintain, I see the documentation as

Please do document the variables with this PR. If not now, when? In my experience, most developers are terrible at writing and maintaining documentation, but its often the difference between good and great software. I'm a great believer in the idea of ubiquitous language from Eric Evans' DDD, and taking time to figure out the right names for variables and functions - which often flows from writing the documentation as ideas clarify. I've refactored plenty of code (including my own!) in this project after writing the documentation.

Anyway - I'm a bit worried we're talking at cross-purposes here, because from my point of view, this change is a no-brainer. There are really no advantages of LESS variables that I can see whatsoever. So if you still don't agree, maybe it'd be worth chatting over IM briefly? I'm on discord with the same username, or happy to use another platform if you prefer.

I definitely think it's gone off the rails LOL! I'm too slow at messaging and it takes up too much time and energy, or do you mean Discord audio 'chat'? Audio and/or video conveys more information and I do like seeing the people I work with, albeit remotely for the last three years!

My concern is that DOM manipulation via structural styles ( e.g. CSS grid dimensions) occurs in a different context to the application logic, and can get out of sync. I don't know how else to put it? Mebbe grid dimensions are the only example?

But I am concerned about supporting problems which are out of scope. Telling punters their problem is out of scope does not end well! My brother has a great metaphor for this: When you discover an empty toilet roll when your pants are around your knees, it may not be your fault, but it is definitely your problem!

[pvspain]

Ok Misha @mmkal, I think the penny has finally dropped for me! TL;DR: I'm ok to drop LESS completely from this project.

My concern relates to people changing 'structural' styles dynamically, in code, leading to desynchronisation of the DOM as understood by the JS.

Your use case, as I understand, is as a downstream consumer of the module, wanting to statically modify grid-view styles, non?

If we flatten the styles from less to css in what is distributed with the package, all the variables defined in the less disappear, so even the demo in the dev dir would stop working.

As this is a component to be used in other applications, I understand we need to be as un-opinionated and easily 'consumable' as possible - so css trumps less, despite its warts and shortcomings. :) My intention has always been to make the styles modifiable. Apart from baseline defaults where necessary, I don't think styling should be a concern of this module, apart from enabling changes and setting up (hopefully!) useful variables.

If we drop less altogether, it should reduce or even eliminate the need for some variables, as the last definition wins when the styles are evaluated? I think the grid height and width are necessary variables as we want to dynamically set those values whenever a new crossword is dynamically loaded.

Your thoughts?

[mmkal]

@pvspain yes that's roughly the way I was thinking - sorry I didn't explain it well.

css trumps less

Agreed, as an output of this library, but I do think it's fine to use less to produce that CSS - it's just a better developer experience for those that work in this repo. I'm not against dropping less but I don't feel a strong need to.

If we drop less altogether, it should reduce or even eliminate the need for some variables

It might do, but (CSS) variables are still worth using over copy-pasting a value in more than one place.

Anyway, lots of conflicts in this now, I will close and create a new one.

[pvspain]

It might do, but (CSS) variables are still worth using over copy-pasting a value in more than one place.

Agreed @mmkal - DRY!

Anyway, lots of conflicts in this now, I will close and create a new one.

I'll leave styling alone so you can complete your work in peace!

My current focus is removing webpack, handing over to vite, and cleaning out unused packages.