Remove trailing and leading newlines from wordlist#26
Open
dgmstuart wants to merge 6 commits intocherdt:mainfrom
Open
Remove trailing and leading newlines from wordlist#26dgmstuart wants to merge 6 commits intocherdt:mainfrom
dgmstuart wants to merge 6 commits intocherdt:mainfrom
Conversation
- Replace tabs with spaces - Add missing semicolons (prompted by linter)
This gives more useful failure messages
A mixture of tabs and spaces were used: this standardises on spaces
The goal here is to have the tests be able to test the javascript code which is used in the page. In order to do that we need to extract the javascript out to a separate file so that the the QUnit tests can test it. This approach follows the advice in the qunit docs: https://qunitjs.com/intro/#make-things-testable ...but modified to allow running via the command line (and therefore on Travis) by following this stackoverflow post: https://stackoverflow.com/a/51861149
We need to make the behaviour here slightly more complicated, so let's first isolate it and cover it with tests.
For long word lists it's useful to be able to break the list up into sub-sections with line breaks. Previously if there was a line break before or after a word, this would be rendered in the bingo card. This change preserves internal line breaks, but removes leading and trailing breaks. Note that this handles multiple line breaks in a row.
Owner
|
There were originally 2 reasons why I combined the HTML, CSS, and JS into the index file:
It's been using jQuery for pieces for quite some time now, so the latter may not be as important (or it means I should work to eliminate jQuery). I agree that moving it to a separate file helps facilitate testing. I'll think about this a bit. |
Contributor
Author
|
Good motivations both.
Regarding offline usage, since the JavaScript file is referenced with a relative path, it’s still usable locally (if you were to move away from JQuery).
From what I’ve read, isolating the JavaScript is a prerequisite for running QUnit tests on it, but another approach could be to use an end-to-end testing framework like Cypress.
Another alternative would be to have the source code in separate files and use some kind of framework which compiles everything down to a single HTML file.
Thanks for engaging with this: very much appreciated.
Duncan Stuart
…On Tue, 26 May 2020 at 02:16 Chris Herdt < Chris Herdt ( Chris Herdt ***@***.***> ) > wrote:
There were originally 2 reasons why I combined the HTML, CSS, and JS into
the index file:
* Reduce requests to the server
* To facilitate offline use of the page
It's been using jQuery for pieces for quite some time now, so the latter
may not be as important (or it means I should work to eliminate jQuery).
I agree that moving it to a separate file helps facilitate testing. I'll
think about this a bit.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (
#26 (comment)
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAC77T6SKVC2GHNHUVIYJHDRTMC53ANCNFSM4NIVGXHQ
).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For long word lists it's useful to be able to break the list up into sub-sections with line breaks.
Previously if there was a line break before or after a word, this would be rendered in the bingo card (see image).
This change preserves internal line breaks, but removes leading and trailing breaks.
Please see the individual commit messages for a description of each step.
In order to test this with Quinit it was necessary to extract the javascript out into its own .js file. I'm happy to split this PR into two if that makes things easier? (one for the test setup, one for the change).