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.

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

Omnibus of new features and fixes #42

Merged
merged 65 commits into from
Oct 12, 2023
Merged

Omnibus of new features and fixes #42

merged 65 commits into from
Oct 12, 2023

Conversation

pvspain
Copy link
Contributor

@pvspain pvspain commented Oct 8, 2023

Apologies for the size!

I think the only outstanding TODO is to improve accessibility.
Changes in reverse chronological order:

Fix: Relocate keybinding to precede gridView construction
Fix: Revert cell id to '(x,y)'
Relabel API buttons in demo: Test -> Check
Add documentation and samples for .ipuz puzzle format.
Fix: setCellAnswer() - don't clobber existing
Add documentation of puzzle format .puz
Fix: Improve demo load time
Package: Add script to bootstrap project and tools from a freshly cloned git repository in a Linux environment.
Add test sample and doc for grid resizing.
Feature: Markdown support in clueText.
Refactor controller construction. Improve documentation.
Add error handling for CrosswordDefinition conversion. Refined crossword domain documentation
Fix: Change usage of event.keyCode to event.key Event.keyCode is deprecated! https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode
Doc: Miscellaneous small tweaks
Feature: Customisable keyboard shortcuts and actions.
Fix: conditionally set currentCell in event:focus
Doc: Improve samples section (crossword-domain.md)
Add default keybindings
Feature: (Re)set crossword file on live controller.
Package: Add project bootstrap script for Linux.
Simplify controller property names.
Refactor: Restructure source: separate files for gridview and cluesview
Refactor: Extract methods and helpers from Controller to reduce complexity.
Refactor: Extract functions to reduce complexity
Package: Add package script "update". Update nvm, npm, node LTS and packages to latest versions. Sort package scripts.
Fix: definition for clueModel.lengthText
Revise clue model regexes to allow whitespace. Extend testing. Extend doc and modify for changes.
Doc: Refine docs for clueModel and crosswordDefinition
Improve crossword validation
Feature: Dynamically load new puzzle
Feature: Expose controller.setCell() to programmatically set grid cell text
Doc: tweak description of TAB keystroke.
Refactor: Refactor CellMap to optimise performance modified: src/cell-map.mjs modified: src/crossword-controller.mjs
Refactor: Extract memoized functions
Refactor: Extract method #delayPublish Remove method context from assert() calls - given in stack dump.
Fix: Processing of (focus,click) event sequence
Fix: Add moveBackCell() - direction of movement now based on current clue.
Refactor: Renaming: anchor:non-anchor -> head:tail
Doc: Improve domain concepts documentation. Change code to match.
Fix: Handle absent clue.solution
Feature: Add 'incomplete' events for clue and crossword testing.

Add prettier support. Play nice with eslint!
revert version

tweak markup

Address lint issues and convert CellMap to class

Address some lint issues

Branch for conversion to ES modules.
Compiling but no crossword visible in sampleapp.
eslint error: "Window is not defined"  src/index.js:13:3

Eslint not working without mjs file suffix on sources.
Dev server (npm start) failing with mjs suffix on src/index.mjs.
Broken broken broken.
Simple node server example.
wip. Framing page displaying.
TODO: wire up crossword elements.

*package.json*

- Add *.less to prettier filetypes.

*crossword.less*

- Tweak revealed and incorrect css - don't process pointer events.
- Cell styles working.

*crossworddom-helpers.js*

- Finished implementation of revealCell.
- Add implementations for testCell and testClue.

*crossworddom.js*

- Add accessor gridElement.
	- can't access private class property in updateCrosswordFontSize, and better encapsulation!
- Add helper function incorrectElement.
- Implement testCurrentClue, testCrossword.
- Add cellIncorrect element inside #createCellDOM

Ready to test new features!

wip.

Rename 'insertStrAt' as 'replaceStrAt'.
Add extensive tests for 'replStrAt'.

*crossworddom-helpers.js*

- Export resetCell and resetClue
- Sort module exports

*crossworddom.js*

- Implement resetClue, resetCrossword.
- Sort module imports.

css fixup
- Adapt memoize() to work for functions _without_ boolean, string or number arguments. Attach random ids to cached objects
-  Add utility toHexString(). Unfortunately this doesn't work for circular (self-referential) objects - like ours!

*crossworddom.js*

- Add name consts for keycode values to  improve code readability.
- Add trace for clue.code in currentClue setter.
 *crosswords.less*

 - Remove bottom,right style attributes. Irrelevant when height,width = 100% (?)

*crossworddom-helpers.js*

- Add hideElement, showElement

*crossworddom.js*

- HideElement calls added to event handlers for BACKSPACE, DEL and KeyPress
Add documentation for helpers# This is a combination of 5 commits.

*helpers.js*

- Add doc for assert()

*crossworddom.js*

- A few non-functional tweaks!
- Refactored TAB key processing - removed loop.

Add api test buttons to sample app. Not working. wip.

Add doc for memoize.

Add crosswordDom to $scope.
Change references in new handlers.
Not working

change event binding syntax
…nts.

- Spell checking (en_GB) for all source files.
- Add JsDoc documentation for significant methods/functions and many helper functions.

*Files*

- src/celldom-helpers.js -> src/cell-element-helpers.js
- src/compile-clue.js -> src/clue-model.js
- src/compile-clue.specs.js -> src/clue-model.specs.js
- src/compile-crossword.js -> src/crossword-model.js
- src/compile-crossword.specs.js -> src/crossword-model.specs.js
- src/crossworddom.js -> src/crossword-controller.js
- src/crossworddom-helpers.js -> src/crossword-controller-helpers.js
- src/celldom-helpers.js -> src/cell-element-helpers.js

*src/crossword-controller.js*

- CrosswordDOM ->  CrosswordController
- #crosswordGridElement ->  #crosswordView
- gridElement -> crosswordView

- Fix bug in #onWindowResize()
  - crosswordView must be extracted from DOM in window event handler

*src/crossword-model.js*

- buildCellArray2D() -> buildCellGrid()
- compileCrossword() -> newCrosswordModel()
- crosswordDefinition -> jsonCrossword
- clueDefinitions -> jsonClues

*clue-model.js*

-  compileClue() -> newClueModel()
Renamed folder node-sample -> sample-node

order script targets alphabetically

Add cspell to package.json

- Add spell-checking to project.
- Fix remaining typos!
- Bump webpack version to address critical issue.
- Prettify crossword samples
*README.md*

- Modify the example code for initialising the package.
- Add example code for using the new API methods.
- Add spell-checker instructions to the Developer Guide.
- Add Design Overview section - reference for MVC pattern.
-  Tweak TODOs

Renamed folder node-sample -> sample-node.
wip.

Fix typos, md formatting
- Re-installed packages.
  - webpack updated for high vulnerability
…ethods.

Add spelling exceptions.
Expand replaceStrAt() to support negative indexes (relative to end).
Add solution support methods cleanCell(), cleanClue().

*README.md*

- Tweak comments in code blocks.
- Add missing keystrokes to _Keyboard Functionality_.

*cspell.json*

- Remove misspelled testing words.

*package-lock.json*

- Update packages

*src/clue-model.specs.js*

- Flag misspelled words for cspell.

*src/helpers.js*

- Extend replaceStrAt() to support negative index values, a la `string.slice()`.

*src/helpers.specs.js*

- Add extra tests for negative index values in `replaceStrAt()`.

*README.md*

- Add documentation for new API methods cleanCurrentClue and cleanCrossword

*src/crossword-controller-helpers.js*

- Extract common logic as setCellText()
-  Change `testCell()` and `testClue()` to return boolean result of test
- Add `cleanCell()` and `cleanClue()`.

*src/crossword-controller.js*

- Add `cleanCurrentClue()` and `cleanCrossword()`
Rename 'sample-node' to 'dev'
Update README with dev server instructions
Package version updates

Add request tracing.

Fix references to sample directories.
Refactor server.js. wip.

- rename `sample-node` to `dev`

*package.json*

- add `nodemon` for live reload of node app on source change.
- extend all **scripts** targets to cover `dev` folder.
- add `"dev"` **scripts** target to start a live-reload dev server.

*dev/server.js*

- a minimal node dev server
- dynamic `.less` file compilation
- static file caching

*sample/sampleapp.js*

- shortened variable names

*src/crossword-controller.js*

- add `tracing` call to control logging from within this file.
- fix: re-add `assert` to imports.

*src/crossword-model.js*

- revert function name to `compileCrossword`

*src/helpers.js*

- add `tracing` function to toggle logging form other source files.

*src/index.js*

- revert to `compileCrossword` for clarity of intent

*README.md*

- Rework and expand _Quickstart_ instructions.
  - Add instruction to run Angular sample
- Add instructions for development server to _Developer Guide_.

Add 'spell:changed' script  to package.json.
Minor README tweaks.

Add dev/favicon.ico

- Add nodemon config (to watch dev/index.html) .
- Add dev/crosswords.js (package public API)

- Add favicon.ico and crosswords.js.
 - Add API button stubs.

Add API button stubs (broken)

wip.

wip

Added yaml crossword.
Add helpers.js (tracing).
wip.

package lock tweaks
Add esm module

conversion to es6 modules.wip

Tests passing.

Add solution querying support. Change from simple javascript devserver to ViteJS and ESM.

Fixup index.mjs reference
Add type module to package.json for ESM.
Add test to incuded dirs for lint, prettier, spell.
Recreate ESM version of require to import JSON crossword definitions.
Commented alternative approach using JSON.parse with embedded await!

modified:   test/crossword-model.specs.mjs

Move unused imports into commented section

modified:   webpack.config.js

Modify for ESM

- css ; font sizes changed to 'rem' from 'em' to avoid 'compounding' issue: https://developer.mozilla.org/en-US/docs/Web/CSS/font-size
- Converted dev server to ESM

Rename compileCrossword to newCrosswordModel
API (crossowrds.js) maintains 'compileCrossword'

**modified:   .eslintrc.yml**
- Changed test script spec for ESM (*.mjs)
**deleted:    dev/server.js**
- Now using `vite` as dev server
**modified:   package-lock.json**
**modified:   package.json**
- Add **viteJS** as dev server and bundler
- Change `dev` script entry to use `vite`
- `chai` moved back to dev dependency as a result of `.eslintrc.yml` change
- Remove `nodemon` with change to `vite` - vite reloads on source change.
**modified:   src/crossword-controller.mjs**
- Add `trace` call for contructor
**modified:   src/crossword-model.mjs**
- Add `trace` call for `newCrosswordModel`

**modified:   dev/index.html**
- Add include of index.js for startup logic
**new file:   dev/index.js**
- Startup logic
**modified:   dev/crosswords.js**
- Convert to ESM
**modified:   src/crossword-controller.mjs**
- Working through seerver/client logic for event handlers. wip.

**deleted:    dev/crosswords.css**
- Dynamically generated by `vite` now
**modified:   dev/index.html**
- Trying onClick setting when DOM ready. Event is firing but `this` and window.controller are not aligned in handler??
**modified:   dev/index.js**
- Move onCLick setting over to index.html
**modified:   package.json**
- Add new dev targets: preview and build
**modified:   sample/sampleapp.js**
- Adjust event handler names. Not working. Crossword grid is HUGE now :(
**modified:   src/crossword-controller.mjs**
- Use `addClass` and new `addClasses` wherever class names set.
- Lots of instrumentation in `revealCurrentCell`. Not working.
**modified:   src/crossword-model.mjs**
- Variable name corrected. nfc.
**modified:   src/crosswords.less**
- Change variables to less-style
- Change font-sizes unit from `rem` back to `em`. Reduce the number of variables.
- Disable  `.cwcell:after` styling since cells are not square now!
**modified:   src/helpers.mjs**
- Add `addClasses`

- Controller event handlers are now firing.
- Issues remain in controller code - current cell index seems off when event fires?
**modified:   dev/crosswords.js**
- Add helpers to export
**modified:   dev/index.html**
- Drop jQuery import
- Drop crosswords.js import - imported indirectly via index.js
**modified:   dev/index.js**
- Add document `DOMContentLoaded` for post-DOM-load logic
- Add event listeners (finally working!!)
**modified:   src/crossword-controller-helpers.mjs**
- Replace intances of `replStrAt` with `setLetter` so we get padding for null/undefined clue.answers
**modified:   src/crossword-controller.mjs**
- Add elementEventHandlers
- TODO: Do we still need all the bind() calls?

**modified:   src/crossword-controller-helpers.mjs**
- Add trace messages to `revealCell`
- Test cell argument to `setCellText`
- Extract inner function `adjustClue`
- Remove styling changes from `setCellText` - outside scope of method!
**modified:   src/crossword-controller.mjs**
- Fix bugs in `revealedElement` and `incorrectElement` -> div index is conditional on clue label in cell
- Add context to `currentCell` and `testCurrentClue` trace message
- Fix bugs in all test/clean/clear/reveal api helper method calls -> controller argument was missing.

Replace `.map()` with `.foreach()` where resultant array is not used.

**modified:   src/clue-model.mjs**
- Bugfix: `clue.solution` normalisation regex was not replacing all instances.
- `clue.answer` normalisation replaces all illegals with spaces and pads to `answerLength`
- Undefined `clue.answer` get padded to `answerLength`
**modified:   src/crossword-model.mjs**
- Tweak doc comments
- Add logic to validate clue solutions
- Add logic to allow for normalised ciue answers and clue solutions
**modified:   test/clue-model.specs.mjs**
- Add tests for normalisable clue answers and soultions
- Add test for undefined solution ('solution' field present but undefined value)
**modified:   test/crossword-model.specs.mjs**
- Add coherence tests for provided solution
- Add coherence tests for provided normalisable solutions

- Clue coherence testing sorted, but display bug introduced
  - answers going in wrong cells  :( WIP!
**modified:   dev/crosswords/ftimes_17095.json**
**modified:   dev/crosswords/ftimes_17095.yml**
**modified:   test/test-crosswords/ftimes_17095.json**
- Bugfix: Coordinates of clue 27d.
**modified:   dev/index.js**
**modified:   src/helpers.mjs**
- Case error in forEach.
**modified:   src/crossword-model.mjs**
- Add more context to exception text for clue coherence tests
**modified:   test/crossword-model.specs.mjs**
- Add test to validate all test crosswords.
- Adjust exception text for clue coherence tests

**modified:   dev/index.js**
**modified:   src/helpers.mjs**
- Fix case in calls to `array.forEach`
**modified:   dev/crosswords/ftimes_17095.json**
- Fix coordinates for 27d in ftimes crossword.

Bugfix in `testClue` and additional tracing
**modified:   dev/index.js**
- Sort element order (group, alphabetic-name)
**modified:   src/crossword-controller-helpers.mjs**
- Add more tracing
- Bugfix: `testClue` - remove short-circuit execution of `testCell`
**modified:   src/crossword-controller.mjs**
- Add more tracing

Bugfix: File-based-answer display. Git template.
new file:   .git-commit-template.txt
- Add a project git commit message template
modified:   README.md
- Add git commit message template installation instructions
modified:   dev/crosswords/ftimes_17095.json
- Add more test answers
modified:   cspell.json
- Add mispelled words in test crossword answers
modified:   src/clue-model.mjs
 - Relocate answer assignment in 'newClueModel' as its now
   dependent upon answer length for padding length
- Do not substitute spaces for illegals in solution normalisation,
  just delete. We want to fail if removing illegals shortens solution.
  This wll flag an exception for crossword setter.
modified:   src/crossword-model.mjs
 - Bugfix: 'newCrosswordModel' was clobbering existing cell value when
   no answer was defined for a clue.
modified:   test/clue-model.specs.mjs
- Add tests for undefined soution and normalised answers and solutions.
- Modify test to expect padded answers for short or undefined
  answers and solutions.
modified:   test/crossword-model.specs.mjs
- Add valid case for coherence test.
- Add valid test for coherence of normalised solutions.

Bugfix: test crossword. Add trace detail and tests
modified:   test/test-crosswords/ftimes_17095.json
- Bugfix: 25d coordinates
modified:   dev/crosswords/ftimes_17095.yml
- Regenerate YAML
modified:   src/crossword-controller-helpers.mjs
- Modify 'adjustClue' to return modified copy of clue.
modified:   src/crossword-model.mjs
- Add detail to error message for coherency error
modified:   test/crossword-model.specs.mjs
 - Add test to validate all test crosswords.

Bugfix: vertically displaced .cwcell-incorrect (displaced to row below)

Bugfixes: Broken crossword-level event handlers
modified:   README.md
- Tweak git commit template instructions
modified:   src/crossword-controller-helpers.mjs
- Commented cell-level trace messages to reduce noise levels in console
modified:   src/crossword-controller.mjs
- Bugfix: Remove trailing space in labels for 'clean-crossword'
          and 'reset-crossword'
- Re-order event handlers - sorted by (group, alphabetical label)
modified:   src/crossword-model.mjs
- Commented trace messages to reduce noise levels in console
Test CSS Grid layout
modified:   package.json
new file:   test/index.html

Moving CSS Grid layout into 'crosswords.less'.wip.

modified:   dev/index.html
- Change grid parent element id to 'crossword-grid'
modified:   dev/index.js
- Add browser styles reset CSS
modified:   src/crossword-controller.mjs
- Style current cell, so colour remains when grid loses focus
modified:   test/index.html
- Fix for grid dimensions to minimally contain cells

Move style files (css/less) into 'style' folder
deleted:    src/crosswords.less
new file:   style/crosswords.less
- Change grid parent element id to 'crossword-grid'
new file:   style/reset.css

Bug: Cells are added in row-order, but are being
dispayed with the rows displaying vertically? (i.e. as columns)

modified:   README.md
- Tweaked controller event descriptions to align with 'dev/index.js'
- Minor styling tweak elsewhere.
modified:   dev/index.js
- Added more documentation.
modified:   src/crossword-controller.mjs
- Add tracing to 'newCellElement' to validate divs are created in
  row order
modified:   style/crosswords.less
- Flipped declaration order of grid rows and columns to try to address
  display bug. No change :(

Fixed nesting
modified:   style/crosswords.less

Fixing css-grid-based crossword grid. wip.
modified:   dev/index.html
- Stripped back to minimal page and rebuilding
new file:   docs/img/css-flexbox-poster.png
new file:   docs/img/css-grid-poster.png
new file:   docs/styling.md
- Add CSS Grid and Flexbox reference documentation
modified:   src/crossword-controller.mjs
- Strip out rows from crossword view creation.
  The grid is created form styles
modified:   style/crosswords.less
- Strip out styles to essentials till gris is working.
  Ref to test/index.html (npm run cssgrid)
modified:   test/index.html
- Simplify page. Commented font attributes belong in input styles

modified:   .gitattributes
Add .png as binary file type

Grid base layout working well. Cell layers are wip.
modified:   dev/index.js
- Bugfix: validate return value of 'document.getElementById()'
modified:   src/crossword-controller.mjs
 - Change implementation of 'set currentClue()'.
   Preserve current cell if contained within new current clue.
   Otherwise set current cell to first cell of new current clue.
modified:   src/cell-element-helpers.mjs
 - Remove explicit setting of current cell in 'toggleClueDirection()'
modified:   style/crosswords.less
- Grid base layout working well. Cell layers are wip.

Fix: Clue label location.
modified:   src/crossword-controller-helpers.mjs
 - Disable font resizing on window scaling/resizing
modified:   style/crosswords.less
 - Correct clue label position by setting cell display to grid
   and justify and align cell-items to start.
 - Input hidden temporarily till grid appearnace sorted.
  - Grid row-line thickness is double column-line thickness. Why?

Tweaking sizes. Looking pretty good.
modified:   style/crosswords.less
- Cell and input dimensions are matched at this ratio of
  grid cell size : input font size
- Switched to 'large' font-size at crossword level

Group colours into a theme. Sizes are relative.
modified:   style/crosswords.less
- Add suffix 'color' to all color variables.
- Cell label positioned correctly.
  Needs a little padding top and left min.
  Currently label slightly overlaps W in cell text.
- Cell overlaps horizontal grid lines and container padding.

Style input focus
modified:   style/crosswords.less
Reintroduce header and buttons.
modified:   dev/index.html
- Revealed marker displaying properly
modified:   style/crosswords.less
- Add body background color to reduce brightness!
- Tweaked revealed size marker smaller
- Introduced display:grid to label and revealed
- Pushed label to top left and revealed to bottom right
- Fixed bug in incorrect diagonal bands
- Otherwise, incorrect is a mess. A full cell-width line of height 0.

Incorrect styling now working
All unneccesary styling removed
modified:   dev/index.html
modified:   style/crosswords.less

Remove unused elements. Word-separators visible
modified:   style/crosswords.less

Remove redundant code (updateCrosswordFontSize).
Crossword sizes all based on ems, so font size
adapts automagically.

modified:   src/crossword-controller-helpers.mjs
- Remove defintion of updateCrosswordFontSize.
modified:   src/crossword-controller.mjs
- Removed windowResize listener and all refs to
  updateCrosswordFontSize.
modified:   style/crosswords.less
- Modify transitions

Move first clue selection to end of constructor.
modified:   src/crossword-controller.mjs
- Remove 'onPageLoaded' event listener.
  Event occurs before controller is constructed.

modified:   cspell.json
- Add less filenames.
modified:   dev/index.css
- Add flexbox styling for body of dev/index.html
- Clues will be side-by side with grid if width permits,
  them will wrap to below grid as width reduces
- Change size units from 'em' to 'rem'
modified:   dev/index.html
- Create two major blocks in body
  '
modified:   dev/index.js
modified:   style/crosswords.less
new file:   style/cwcolors.less
new file:   style/cwdimensions.less

Addition of info from cw def using simple js.
chore: Introduce pug html template engine.
modified:   dev/index.html
modified:   dev/index.js
- Trial addition of information from crossword definition
  using only javascript and 'span' element injection via
  element id. Limited results as we can't populate anchor
  tag href strings using span elements.
  Switching to 'pug' html template engine. This will also
  affors us looping and condtionals in template code.
modified:   .eslintrc.yml
- Add vite config to 'import/no-extraneous-dependencies'
modified:   package-lock.json
modified:   package.json
- Add pug for html templates in 'dev'
new file:   vite.config.js
- Add pug to plugins
deleted:    nodemon.json
- No longer used since vite introduced

Adnd html template docs. jQuery no longer used.
new file:   docs/html-templates.md
- Add dcumentation for HTML tempalte engine pug.
deleted:    dev/vendor/jquery/jquery.js
deleted:    dev/vendor/jquery/jquery.min.js
deleted:    dev/vendor/jquery/jquery.min.map
modified:   package.json
- Remove jQuery

Add titel block template. Pug integration broken.
modified:   dev/index.html
modified:   dev/index.js
new file:   dev/template/title-block.pug
modified:   vite.config.js
 - Add code for JS injection into Pug templates.
   Not working. Time to read up on Vite config!

Pug import broken. Add Vite docs.
modified:   dev/index.html
- Move reset.css into dev folder
- Move title-block.pug into dev folder
renamed:    style/reset.css -> dev/reset.css
- Unreachable from dev/index.html when not under dev
renamed:    dev/template/title-block.pug -> dev/title-block.pug
- Add variable referencing
new file:   docs/vite.md
- Links to resources for Vite and vite-plugin-pug
modified:   package-lock.json
- version updates
modified:   vite.config.js
- Flattened 'locals'

Minor changes
modified:   src/crossword-controller.mjs
- Silence tracing from #newCEllElement
modified:   vite.config.js
- Load crossword via import statement

Trialing 'vite-plugin-pug-transformer'. Broken.
modified:   dev/index.html
- Change element for pug import
new file:   jsconfig.json
- TypeScript config file for a JS project as described here:
   https://www.typescriptlang.org/docs/handbook/tsconfig-json.html
   Including TS may get things working?
modified:   package-lock.json
modified:   package.json
- Add 'vite-plugin-pug-transformer' and 'typescript'
modified:   src/crossword-controller.mjs
- Add sanity checks for 'CrosswordController' constructor args.
modified:   vite.config.js
 - Change to use hardcode strings for crossword info data in pug template
Add crossword clues DOM element. Working. Styling to come.
modified:   dev/index.html
- Disable title info template for now.
  Needs to be populated dynamically when crossword changes.
modified:   dev/index.js
- Add crosswordCluesParent assignment
modified:   src/crossword-controller.mjs
- Add #crosswordCluesView
- Rename #domParentElement -> #domGridParentElement
- Add method #newCrosswordCluesView
- Clues appearing.
- Click event handlers are firing.
modified:   style/crosswords.less
 - Changes to include crossword clues
 - Start styling for clue block

Working on layout. wip.
modified:   dev/index.css
- Add flex to groups. wip.
modified:   dev/index.html
- Added div 'groups' to wrap groups.
- Can be removed and styles moved up to body.
modified:   dev/index.js
- Adjust for changed element ids in dev/index.html
modified:   dev/reset.css
 - Formatted. nfc.
modified:   package.json
- Change prettier rules to process style files under dev
modified:   src/crossword-controller.mjs
- Add div 'title' to crossword clues view.
- title will span full-width, with clue gropus below
- Change some ids
modified:   style/crosswords.less
modified:   style/cwdimensions.less
- Move grid sizes into style/cwdimensions.less
modified:   style/cwcolors.less
- Harmonising 'current' element colours across grid and clues. wip.

More clues styling. wip.
modified:   dev/index.html
modified:   dev/index.js
- div id change
modified:   src/crossword-controller.mjs
- Remove title, add wrapper (for different flex of clue groups to block)
- Nasty hack to get clue text to align after clue label of varying
  width. Replace with separate elements for label and text,answer-length
modified:   style/crosswords.less
- Introduce flex wrap to clue groups so down wraps under across as
  display width shrinks

Behaving as three groups now.
modified:   dev/index.css
modified:   dev/index.html
- Outermost group (page-wrapper) is a row-oriented flexbox.
  It contains 3 columns:
  * first-column - column-oriented flexbox
    title-block
    crossword-grid-wrapper (grid parent element)
    current-clue
  * second-column - row-oriented flexbox with wrap and width min-content
    //TODO: Better as column-oriented flexbox?
    api-block
  * third-column - row-oriented flexbox with wrap
    crossword-clues - (clues parent element)
modified:   dev/index.js
- change ids of parent elements for grid and clues
modified:   src/crossword-controller.mjs
- simplify construction in #newCrosswordCluesView
modified:   style/crosswords.less
- crossword-clues-block renamed to crossword-clues-wrapper

Adjust layout
modified:   dev/index.css
modified:   dev/index.html
modified:   dev/index.js#
Break clue element into collection of three spans
modified:   src/crossword-controller.mjs
- Extract method addClueElements
  Break clue element into collection of three spans,
  so we can style each clue paret separately.

Clue text aligned
modified:   src/crossword-controller.mjs
- Combine clue text and answer length into one element
modified:   style/crosswords.less
- Fix reference to class 'crossword-clues'
- Style clue-label as display:inline-block to set overall label width
Handle clueSelected for controller.#crosswordCluesView
modified:   src/crossword-controller.mjs
modified:   style/crosswords.less
- Resolve multiple import problem (move to global scope)
- Add styling for selectec clues
modified:   style/cwdimensions.less
 - Reinstate dimemsion for clue label width

Clue view now responding correctly to current clue
changes in model.
modified:   dev/index.css
- Change body background colour
modified:   src/crossword-controller.mjs
- Add modelClue property to clueElement to allow
  comparison of current model clue to clue associated
  with view clue in onStateChanged handler
modified:   style/crosswords.less
- Add padding to clues
modified:   style/cwcolors.less
- Rationalise and rename colours so we have
  @cell-current-color and @clue-current-color rather than
  confusing adjectival colour names.
…sub, controller.addEventListener

Current clue element working in dev.
Todo: wrap currrent clue at grid width.
modified:   dev/index.css
- Tweak styling of #current-clue
modified:   dev/index.html
- Remove placeholder text for #current-clue
modified:   dev/index.js
- Hook up #current-clue to 'clueSelected' controller event
modified:   sample/sampleapp.js
- Change to use controller.addEventListener
modified:   src/crossword-controller.mjs
- Add enumeration controllerEvents
- Relocate initialisation of simple CrosswordController private
  members to declaration point
- Add pub-sub facilty to CrosswordController
- Wrap pub-sub as getter addEventListener
- Change implementation of #stateChange to use pub-sub
- #newCrosswordCluesView now uses pub-sub
modified:   src/helpers.mjs
- Add newPubSub implementation as a helper
modified:   style/crosswords.less
- Thin word separators to 1px
- Remove redundant import declarations
modified:   dev/crosswords/ftimes_17095.yml
 - Convert extended characters to UTF-8 equivalents
modified:   dev/index.css
- Add title-block padding
modified:   dev/index.html
- Move page heading out of title-block
- Add remaining info content to title-block
modified:   dev/index.js
- Group local functions at top of DOMContentLoaded
  event callback
- Extract eid() function
- Populate new title-block elements with json content
modified:   src/crossword-controller.mjs
- Simplify and remove redundant code for answer
  segment terminators
- Add missing clue argument to #stateChange call in
  focus event callback
modified:   style/crosswords.less
- Add styling for case of a cell being both an across
  and down word separator
modified:   dev/index.css
- Mirror styles for clues in styles/crosswords.less
modified:   dev/index.html
- Mirror structure of clues in CrossowrdCluesView for
  current clue element
modified:   dev/index.js
- rewrite logic for two span elements for current clue
modified:   src/crossword-controller.mjs
 - bugfix: typo caused unknown property - failing silently
   as undefined reference.
deleted:    dev/crosswords/ftimes_17095.yml
deleted:    dev/crosswords/guardian_quiptic_89.json
deleted:    test/test-crosswords/alberich_4.json
deleted:    test/test-crosswords/ftimes_17095.json
deleted:    test/test-crosswords/quiptic89.json
renamed:    dev/crosswords/alberich_4.json -> data/alberich_4.json
renamed:    dev/crosswords/ftimes_17095.json -> data/ftimes_17095.json
new file:   data/ftimes_17095.yml
renamed:    src/data/quiptic89.json -> data/quiptic_89.json
    Relocate crossword definition files to 'data' folder
modified:   README.md
    Adjust references to crossword definition files.
modified:   cspell.json
modified:   dev/index.js
modified:   docs/controller-api.md
    Add TOC, section on adding event handlers
renamed:    docs/crossworddefinition.md -> docs/crossword-definition.md
renamed:    docs/crosswordobject.md -> docs/crossword-object.md
new file:   docs/crossword-styling.md
    Add doco for module styling
deleted:    docs/html-templates.md
    Remove pug artifact
modified:   docs/vite.md
modified:   package.json
    Add yml files to prettier
modified:   sample/sampleapp.js
modified:   src/clue-model.mjs
modified:   src/crossword-controller.mjs
    Add 'this' binding to controller 'bind*' methods
modified:   test/crossword-model.specs.mjs
    Adjust references to crossword definition files.
modified:   cspell.json
    Add exception
modified:   data/quiptic_89.json
    answer property renamed as solution
new file:   docs/README.md
    Contents file for docs directory
modified:   docs/controller-api.md
    Minor tweaks. nfc.
modified:   docs/crossword-definition.md
    Minor tweaks. nfc.
new file:   docs/crossword-model.md
    Add preliminary description of clue data structure.
    Expand cell description.
deleted:    docs/crossword-object.md
    crossword-object.md renamed as crossword-model.md
modified:   docs/crossword-styling.md
    Add more references - attribute reset.css
    Extract "Suggested configuration" section
modified:   src/clue-model.mjs
    Refactoring: extract helpers for newClueModel()
    to reduce complexity.
    Refactoring: use string.endsWith() where appropriate
modified:   src/crossword-model.mjs
    Refactoring: extract helpers for newCrosswordModel()
    to reduce complexity.
    Refactoring: use for..of loop where appropriate
modified:   test/crossword-model.specs.mjs
    Change terminology from 'non-linear' clue to 'multi-segment' clue
Add doc sections on grid and clue view elements.

modified:   dev/index.js
    Fix crossword location.
modified:   docs/README.md
    Re-order content and modify file descriptions
modified:   docs/crossword-definition.md
    Remove unclear references to segment ordering.
modified:   docs/crossword-model.md
    Fix bug in description of clueModel
modified:   docs/crossword-styling.md
    Rename 'Dev page' section as 'Crossword clues'
renamed:    docs/controller-api.md -> docs/module-api.md
    Reformat and changed section names.
    Add sections on grid and clue view elements.
    Add EventData column to controller events table.
    Add links to styling docs
modified:   src/clue-model.mjs
    Shorten args in for..of loops.
    Refactor directionFromClueLabel() as nested function
    in buildConnectedDirectedClues().
    Remove bug in description of clueModel.solution
modified:   src/crossword-controller.mjs
    Pass 'data' to listeners where absent.
    Refactor isCurrentClueSegment().
    Extract addEventListeners() helper for #newCellElement()
    Refactor logic in 'keydown' event listener to switch statement
modified:   src/crossword-model.mjs
    Extract helpers for newCrosswordModel()
modified:   sample/sampleapp.js
    Fix crossword location. Minor refactor.
modified:   README.md
    Add links to documentation pages.
    Minor content and format changes.
    Update TODOs with completed items
modified:   docs/keyboard-shortcuts.md
    Add default DELETE shortcut.
    Correct behaviour of BACKSPACE
    Minor content tweaks.
modified:   docs/module-api.md
    Fix type of CrosswordModel
modified:   README.md
modified:   docs/README.md
renamed:    docs/crossword-model.md -> docs/crossword-data-structures.md
    Renamed crossword-model.md -> crossword-data-structures.md
modified:   docs/module-api.md
    Prettier reformat
modified:   docs/crossword-definition.md
deleted:    sample/crosswords/alberich_4.json
deleted:    sample/crosswords/ftimes_17095.json
deleted:    sample/crosswords/guardian_quiptic_89.json
    Redirected crossword samples under sample/ to data/
Merged 49 commits into dwmkerr:main from pvspain:develop
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 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 publishing 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

modified:   .eslintrc.yml
new file:   .git-commit-template.txt
new file:   .gitattributes
modified:   .gitignore
new file:   .mocharc.json
modified:   CHANGELOG.md
modified:   README.md
new file:   cspell.json
new file:   data/alberich_4.json
new file:   data/ftimes_17095.json
new file:   data/ftimes_17095.yml
renamed:    src/test-crosswords/quiptic89.json -> data/quiptic_89.json
new file:   dev/crosswords.js
new file:   dev/favicon.ico
new file:   dev/index.html
new file:   dev/index.js
new file:   dev/index.less
new file:   docs/README.md
new file:   docs/crossword-data-structures.md
new file:   docs/crossword-definition.md
new file:   docs/crossword-styling.md
deleted:    docs/crossworddefinition.md
deleted:    docs/crosswordobject.md
new file:   docs/img/css-flexbox-poster.png
new file:   docs/img/css-grid-poster.png
modified:   docs/keyboard-shortcuts.md
new file:   docs/module-api.md
modified:   docs/screenshot.png
new file:   docs/solution-support.md
new file:   docs/vite.md
new file:   jsconfig.json
modified:   makefile
modified:   package-lock.json
modified:   package.json
deleted:    sample/crosswords/albreich_4.json
deleted:    sample/crosswords/guardian_quiptic_89.json
modified:   sample/index.html
modified:   sample/sampleapp.js
new file:   src/cell-element-helpers.mjs
renamed:    src/cell-map.js -> src/cell-map.mjs
new file:   src/clue-model.mjs
deleted:    src/compile-clue.js
deleted:    src/compile-clue.specs.js
deleted:    src/compile-crossword.js
deleted:    src/compile-crossword.specs.js
new file:   src/crossword-controller-helpers.mjs
new file:   src/crossword-controller.mjs
new file:   src/crossword-model.mjs
deleted:    src/crossworddom.js
deleted:    src/crosswords.less
deleted:    src/data/quiptic89.json
deleted:    src/helpers.js
new file:   src/helpers.mjs
deleted:    src/index.js
new file:   src/index.mjs
deleted:    src/index.specs.js
deleted:    src/test-crosswords/albreich_4.json
new file:   style/crosswords.less
new file:   style/cwcolors.less
new file:   style/cwdimensions.less
new file:   style/reset.css
new file:   test/clue-model.specs.mjs
new file:   test/crossword-model.specs.mjs
new file:   test/helpers.specs.mjs
new file:   test/index.html
new file:   test/index.specs.mjs
modified:   webpack.config.js
modified:   .github/workflows/pull-request.yml
modified:   .github/workflows/release-please.yml
modified:   cspell.json
modified:   dist/crosswords.css
modified:   dist/crosswords.js
modified:   package.json
modified:   src/crossword-controller.mjs
modified:   style/crosswords.less
modified:   style/cwdimensions.less
@pvspain
Copy link
Contributor Author

pvspain commented Oct 8, 2023

Hi Misha @mmkal,

I have included your changes from PR38, but they have been relocated with refactoring of src/crossword-controller.mjs to reduce complexity. The CSS vars are now set inside newCrosswordGridView() in src/crossword-gridview.mjs.

Copy link
Contributor

@mmkal mmkal left a comment

Choose a reason for hiding this comment

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

This looks great from eyeballing it! A small comment about yaml being in dependencies so far.

I can help test this, but I'll need the css variables change to go in first or be included in this branch - I'm relying on that for styling in my app.

@@ -10,6 +10,9 @@
"style"
],
"type": "module",
"dependencies": {
"yaml": "^2.3.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a big dependency, and potentially controversial - on an old team we found js-yaml more performant and smaller.

I think it should be left as an "exercise to the reader" to parse yaml into a json object. That way end users could install their preferred yaml library before passing a plain old JavaScript object to crosswords-js.

No objections to it being a dev dependency for the sake of the demo site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Misha!

This is a big dependency, and potentially controversial - on an old team we found js-yaml more performant and smaller.

I've had a quick look at js-yaml - great tip - thanks!

I think it should be left as an "exercise to the reader" to parse yaml into a json object. That way end users could install their preferred yaml library before passing a plain old JavaScript object to crosswords-js.

No objections to it being a dev dependency for the sake of the demo site.

I understand your concern. Our points of differentiation to date are no dependencies, a small footprint, and universal usage (pure JS).

YAML makes manual document editing feasible. Looking at the JSON in ipuz, for example - there's little chance people would regularly construct a 15x15 puzzle by hand!

Simplicity is the third of @dwmkerr Dave's design goals. I think the puzzle format plays into this. I would like to support YAML for the sake of puzzle makers. Perhaps not in the core repo? I've spoken to @dwmkerr about managing framework samples as separate repos, perhaps another repo for useful utilities, like YAML imports?

Once we move beyond one repo, we should move to an organization account on GitHub, as is the pattern. GitHub handles link forwarding, so we wouldn't lose anyone in the transition.

Copy link
Owner

Choose a reason for hiding this comment

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

If the goal is that the puzzles could be edited by human users without technical knowledge, I think yaml will be more useful than JSON! I love the idea of a YAML editor on the left side of a screen with a puzzle on the right, for live interactive editing. In terms of which library to use, I'm fairly flex, if we use bundlers that do tree-shaking it may matter less - at this stage I'd say keep libraries to the minimum number needed, but don't worry too much about size, if overall size becomes a problem we can optimise later. For now I'd focus on dev experience and user experience!

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's how I see the best of all worlds being achieved:

  • keep all dependencies including yaml out of crosswords-js - which could become the core crosswords renderer package. I am using it (and a big fan!) on a site I'm building now. I don't need yaml because the main use case is crossword playing, not crossword setting. The setters are uploading .puz files which I'm parsing using another library. Yaml would be baggage in this package in my opinion
  • add yaml and maybe some other opinionated dependencies in a package called something like @crosswords-js/set which is designed to help people write new crosswords - this should be separate because it's a whole different set of concerns, I think
  • if there are more spin-off packages for slightly different use cases, those could also go under the (for now non existent) @crosswords-js/ npm scope.

That may sound like scope creep for this project but I would say if that's the case then it'd be better to keep yaml out for now. When we switched to js-yaml on my old team we still had problems keeping it out of our client bundle.

The above idea could either be implemented with another repo like @pvspain suggested or by converting this into a multi-package monorepo - monorepo tooling these days is very good for npm packages. My recommendation would be to do that and use turbo.build. I'd be happy to help set this up if interested.

@pvspain
Copy link
Contributor Author

pvspain commented Oct 9, 2023

This looks great from eyeballing it! A small comment about yaml being in dependencies so far.

I can help test this, but I'll need the css variables change to go in first or be included in this branch - I'm relying on that for styling in my app.

If you want to test these mods, I've written a gist on processing pull requests locally. This method enables you to evaluate changes without interrupting your ongoing work. I use the process regularly before submitting PRs to ensure they go in smoothly.

@dwmkerr
Copy link
Owner

dwmkerr commented Oct 11, 2023

This looks great - I'd say merge and fix forward for any issues, as this is such a large one. Want me to hold off until we've got more clarity on the YAML side of things?

@pvspain
Copy link
Contributor Author

pvspain commented Oct 12, 2023

I vote we merge now and refactor later. Let's keep the velocity up. We don't want a backlog of PR's, as it makes merging later PRs more difficult.

To that end, how about a policy of only submitting PRs that are ready to merge? The PR must merge smoothly into dmwkerr/main, as @dwmkerr has requested previously. The onus is on the submitter to ensure that by test-merging offline. To assist that, I've written a gist to work through the PR locally, which I use for every PR, and also for merging changes back upstream. There are several checkpoints to test and inspect the changes.

Lets have another channel for discussing the roadmap and technology changes? We don't want wasted effort in PR's - nobody's getting paid here! @dwmkerr's original TODO is complete with this PR, apart from improving accessibility; tabbing out of the gridview, for example, is impossible with the default keybindings. I imagine there's prior art for that problem.
Do you want to set up another channel Dave? @dwmkerr

I'm happy with segregation into different modules, a multi-package mono-repo - just no more complicated than necessary right now. We can always refactor. Knock yourself out @mmkal! I have no issue with keeping this core repo dedicated to compiling crossword definitions (JS Objects) and rendering the different views - grid, clues. With the proviso that our hello world remains simple, as does adding YAML support. I think we're all agreed it's a better format for human editing.

On a side note, I've switched to js-yaml from yaml on an upstream branch and its painless. The example yaml puzzles all convert happily. I see the major task with extracting YAML is rewriting the docs, as there are a lot of links and references to YAML now. It will be easier if we have another repo set up before all that editing begins.

@mmkal
Copy link
Contributor

mmkal commented Oct 12, 2023

That's ok with me - maybe as a simpler follow-up to this I could submit a pull request which switches js-yaml to be an optional peer dependency instead. I agree that end users should be encouraged to write yaml over json, I'm just nit-picking implementation details from the perspective of a developer-user rather than a webapp-user.

(And of course, I am just a user of this package giving feedback, so things shouldn't be held up on my account!)

@dwmkerr
Copy link
Owner

dwmkerr commented Oct 12, 2023

Love it - I'm going to merge

@dwmkerr dwmkerr merged commit d9c65a8 into dwmkerr:main Oct 12, 2023
2 checks passed
@dwmkerr
Copy link
Owner

dwmkerr commented Oct 12, 2023

This is amazing work - @pvspain and @mmkal should we start some kind of design doc we can work on offline on how we want to structure things going forwards? I like the idea of an org for this, and maybe modules like:

  • Crossword format loader (responsible for parsing, error-checking and validating json/yaml xword definitions) - able to turn them into the in-memory model. Allows anyone to use 'our' crossword format
  • Crossword builder: takes a loaded crossword model and creates a usable in-memory structure that can power interaction, either server side or client side (e.g. holds the clues, the letters entered, the state of answers, etc)
  • 3x samples - a 'plain old javascript sample' a 'react native sample' then maybe a vue sample or even a server side command line one
  • then a geeky one for me - I'd like a terminal based crossword tool! so I can load a crossword file and play in in the terminal :)

@dwmkerr
Copy link
Owner

dwmkerr commented Oct 12, 2023

We could set a timeline / set or priorities, modularise a bit and then divide and conquer based on interests, what do you think? Could then have code owners per module, e.g. we all own the format/foundational stuff, but divide and conquer on samples?

@dwmkerr
Copy link
Owner

dwmkerr commented Oct 12, 2023

It also democratises the ownership a bit and lets you both move quickly, with my workload I don't want to be a blocker, but would love to give input (and maybe own the dorky commandline version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants