Migrate to ES6, add solution features and add clues-block DOM element

[pvspain]

Hi Dave, long time no PR! I hope you're keeping well?

Sorry to dump a truckload on you. I was trying to submit a cohesive and complete change, but it just kept growing. I've rebased all my commits to reduce the number of changes to you need to review and QA, but there is still a lot!

Here are the big ticket items:

• Migrated the code to ES6 and ESM modules
• Wrote a pure NodeJS demo/test sample in the dev folder
• Switched to ViteJS for bundling and serving of dev
• Added solution support feature - test, reveal, clean, reset of cells, clues and crossword
• Extended crossword definition to support 'answer' and 'solution' properties for each clue
• Solution support features added to dev
• Added (optional) crossword-clues DOM element that syncs with crossword-grid DOM element
• Added spell checking (cspell) to project
• Restyled crossword to use CSS Grid and Flexbox and made responsive
• Parameterised CSS to simplify changing look-and-feel.
• Renamed and re-organised code to follow MVC pattern artefacts
• Added generalised pub-sub to controller (was crossworddom) and publish all state change - events.
• Added a lot of documentation
• Reworked and extended README and added doco links
• Ticked off most of the TODOs in the README

[mmkal]

@dwmkerr @pvspain Very much looking forward to this going in! I'm using it now by installing c074f80024ee162c1f811ec9c30740024420a435 as an npm dependency directly and it works great. The solution support features are excellent.

There are some things I'd love to submit as pull requests once it's in main:

1. add typescript types
2. expose the resetCell and cleanCell functions in crossword-controller-helpers.mjs
3. add a content = ' ' default parameter to the above functions. This allows programmatically setting a cell's content (although maybe there's an existing way to do this? I patched the file, and this enabled online collaboration on crosswords)

A couple of other things I've spotted along the way:

• there are some minor CSS problems on mobile. I've noticed that the borders/outlines are off (iOS/Chrome). I think it's something about outline-offset but hard to be sure
• the CrosswordController class creates some elements with ids (crossword-across-clues and crossword-down-clues). I don't personally have a use case currently where I need multiple crosswords on a page, but if someone did, I can imagine that causing some problems.

Also re https://github.com/dwmkerr/crosswords-js/pull/34#issuecomment-1694133298 - it'd still be great to compile the less as part of package bundling, outside of the Vite sample. I'm using the library in react, and it works well, once I run lessc node_modules/crosswords-js/style/crosswords.less src/styles/crosswords-js-compiled.css - so presumably this could be done in this repo too, outside of the Vite sample.

[pvspain]

Hi Misha,

I'm so pleased you like the new features - thanks!

One of my aims for the project (and I think Dave would agree) is to minimise external dependencies - to avoid dependency hell https://en.wikipedia.org/wiki/Dependency_hell, and to minimise package size and our attack surface.

At this stage, I think we're using enough less features to lock in that choice. I realise these are common features across many CSS preprocessors now. The less project https://lesscss.org/ and community has shown itself to be quite adaptive to evolving preprocessor features over time, and I'm not aware of any missing killer-features, or use-cases in this project to warrant the effort of changing to another preprocessor. I'll add less as a non-dev dependency in my current feature branch.

I couldn't think of use cases to expose resetCell and cleanCell myself, so that's why they aren't currently exported. I'm not sure they should have associated events, as that could result in an event publishing explosion?

The CSS border/outline problem is a real head scratcher! I notice it when you scale the webpage below 100% and at lower viewport sizes - as you mentioned. I've looked at $The Age https://www.theage.com.au/puzzles/crosswords crossword to see how they're implementing grids, and it's html-table-based. I'd rather stick with CSS Grid for separation of concerns https://en.wikipedia.org/wiki/Separation_of_concerns purity(!), but the border inconsistency does offend my aesthetic sense! If you make any progress identifying the cause of the problem and a solution, please share.

Regarding element ids, I've added several performance improvements to the code in my current feature branch, including a rewrite of CellMap, which uses ids on all grid cell div elements (set in #newCellElement). If someone comes up with a multi-crossword scenario on a single page, we could effectively namespace the element ids per crossword with a unique prefix generated by each Controller.

Typescript is on my skill-path, so I'd like to see your work. I had considered a rewrite down the road, but a wrapper as you seem to be suggesting(?) would perhaps be less effort, and doesn't close the door on pure JavaScript. Would you elaborate on your idea please?

Thanks again for your interest Misha - it's great to be able to discuss the project and bounce ideas around!

Cheers, Paul

On Sat, 2 Sept 2023 at 08:09, Misha Kaletsky @.***> wrote:

@dwmkerr https://github.com/dwmkerr @pvspain https://github.com/pvspain Very much looking forward to this going in! I'm using it now by installing c074f80 https://github.com/dwmkerr/crosswords-js/commit/c074f80024ee162c1f811ec9c30740024420a435 as an npm dependency directly and it works great. The solution support features are excellent.

There are some things I'd love to submit as pull requests once it's in main:

1. add typescript types
2. expose the resetCell and cleanCell functions in crossword-controller-helpers.mjs
3. add a content = ' ' default parameter to the above functions. This allows programmatically setting a cell's content (although maybe there's an existing way to do this? I patched the file, and this enabled online collaboration on crosswords)

A couple of other things I've spotted along the way:

• there are some minor CSS problems on mobile. I've noticed that the borders/outlines are off (iOS/Chrome). I think it's something about outline-offset but hard to be sure
• the CrosswordController class creates some elements with ids ( crossword-across-clues and crossword-down-clues). I don't personally have a use case currently where I need multiple crosswords on a page, but if someone did, I can imagine that causing some problems.

Also re #34 (comment) https://github.com/dwmkerr/crosswords-js/pull/34#issuecomment-1694133298

• it'd still be great to compile the less as part of package bundling, outside of the Vite sample. I'm using the library in react, and it works well, once I run lessc node_modules/crosswords-js/style/crosswords.less src/styles/crosswords-js-compiled.css - so presumably this could be done in this repo too, outside of the Vite sample.

— Reply to this email directly, view it on GitHub https://github.com/dwmkerr/crosswords-js/pull/36#issuecomment-1703374642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFO4SYVSCKAPOBHO6VGT4TXYJMINANCNFSM6AAAAAA4CGNJRM . You are receiving this because you were mentioned.Message ID: @.***>

[mmkal]

Replying in reverse order:

TypeScript - absolutely. As of fairly recently TypeScript can now create definitions from JavaScript. So no need to re-write anything. Right now, in my downstream project, I'm actually running tsc using a special tsconfig.json file which points at node_modules/crosswords-js/src/**/*". So it reads the .mjs files in this project and figures out (as best it can) the contracts. I would propose putting something similar as a build-step in this project. Then the typings could gradually be improved over time to yield better DX for users who install crosswords-js. For example, a function like this:

const hideElement = (element) => {
  element && element.classList.add('hidden');
};

would produce typescript like:

const hideElement: (element: any) => void

but we could add simple jsdoc annotations to the javascript file like:

/** @param {HTMLElement} element */
const hideElement = (element) => {
  element && element.classList.add('hidden');
};

then the typescript output would be:

const hideElement: (element: HTMLElement) => void

Added bonus: developers of this repo will get intellisense in most IDEs:

---

Re: CSS border/outline. I haven't been able to get to the bottom of it either. I'm not well-versed in front-end/CSS generally. I only have some workarounds/overrides in my CSS.

This one does seem like a no-brainer, it gets rid of the strange rounded corners of the highlighted cells on mobile (maybe chrome iOS has a default of border-radius: 50% or something).

.crossword-grid input.active {
  border-radius: 0;
}

Another thing that I think would give more levers to pull would be more tagging generally of cells based on the definition. For example the cw-down-word-separator is helpful, but it's on the input element inside a cell, rather than on the cell itself, meaning you have to use the :has(...) pseudo-selector which isn't universally supported. Also from brief playing around I found bottom borders on mobile were invisible, hidden under the next cell, so it'd also be useful to have a class on the cell after a word separator, etc. Maybe some data-* attributes would give more flexibility in how things are styled. Another thought re: styling - CSS variables could be a nice way of allowing end-users to change highlight-colour from green to yellow, or similar. Anyway, adding/removing anything to the CSS design would take more consideration than I've given it so far. It works pretty well for now!

---

Re resetCell and cleanCell. My use case is programatically updating the content of a cell (allow remote collaboration on a crossword - when a player on the other end fills in a cell, I'm sending a websocket message to tell the other player's controller to fill in that cell). There might be another way to do it, but a slightly-modified resetCell did the trick well enough.

Re events: I don't think there needs to be an event for resetCell/cleanCell specifically - but, there may be a cellUpdated event missing for my use case. I'm currently using a gridParent.addEventListener('keypress', ...) event to detect when a cell is filled in. This triggers the websocket.send(...). I tried the clueSelected event at first but it fired frustratingly late and wasn't a good experience. But it'd be great for the CrosswordController to abstract this so I didn't have to attach an event listener to a DOM element directly.

---

Re: less. I don't think it should become a production dependency. The normally way to do this would be to have it as in devDependencies, then "build": "lessc style/crosswords.less dist/crosswords.css" as the build step, to be run before publishing the package to npm. Then people who do npm install crosswords-js can directly access the CSS, no need for them to have less in their node_modules. It'd also allow access via unpkg.com etc. It'd be very unusual to ship less as a dependency, and I really don't think it's necessary.

---

Overall: I think all of the above are nice-to-haves, not essential - again, I would be thrilled for this pull request to go in exactly as is, then I could submit some of the above as follow-on pull requests.