Merge pull request #1351 from klembot/develop

2.6.0

.github/pull_request_template.md

@@ -1,13 +1,12 @@
-🛑 **Only PRs related to localization are being accepted for Twine 2.4 right now.** \
-🛑 **Only PRs that fix bugs will be accepted for Twine 2.3. No PRs that add new features or modify existing functionality will be accepted.**
-
 # Description
 
 Enter a short description of what this PR does.
 
-# Issues fixed
+# Issues Fixed
+
+_If you are contributing a localization, you can skip this section._
 
-Link any issues that this PR resolves.
+Link the issues that this PR resolves. Your PR must resolve at least one issue, and agreement must be reached in the issue on your implementation approach before opening this PR.
 
 # Credit
 
@@ -16,7 +15,7 @@ Please put an X in *one* of the squares below only.
 [ ] I would like to be credited in the application as: ______ \
 [ ] I would not like my name to appear in the application credits.
 
-# Presubmission checklist
+# Presubmission Checklist
 
 Put an X in the squares below to indicate you've done each step.
 

.github/workflows/eslint.yml

@@ -0,0 +1,21 @@
+name: ESLint
+
+on:
+  push:
+    branches: [develop]
+  pull_request:
+    branches: [develop, main]
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v2
+        with:
+          cache: npm
+          node-version: 16
+      - name: Install
+        run: npm ci
+      - name: Link
+        run: npm run lint

.github/workflows/prs.yml → .github/workflows/jest.yml

@@ -1,4 +1,4 @@
-name: PRs
+name: Jest Tests
 
 on:
   push:
@@ -7,17 +7,15 @@ on:
     branches: [develop, main]
 
 jobs:
-  lint-and-test:
+  test:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-node@v2
         with:
           cache: npm
-          node-version: 14
+          node-version: 16
       - name: Install
-        run: npm install
-      - name: Lint
-        run: npm run lint
+        run: npm ci
       - name: Test
         run: npm run test:coverage -- --maxWorkers=2

.github/workflows/playwright.yml

@@ -0,0 +1,28 @@
+name: Playwright Tests
+on:
+  push:
+    branches: [develop]
+  pull_request:
+    branches: [develop, main]
+jobs:
+  test:
+    timeout-minutes: 60
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - uses: actions/setup-node@v3
+      with:
+        cache: npm
+        node-version: 16
+    - name: Install dependencies
+      run: npm ci
+    - name: Install Playwright Browsers
+      run: npx playwright install --with-deps
+    - name: Run Playwright tests
+      run: npx playwright test --reporter=line
+    - uses: actions/upload-artifact@v3
+      if: always()
+      with:
+        name: playwright-report
+        path: playwright-report/
+        retention-days: 30

.gitignore

@@ -5,4 +5,7 @@ dist/*
 .idea
 electron-build/*
 coverage/*
-docs/en/book/*
+docs/en/book/*
+/test-results/
+/playwright-report/
+/playwright/.cache/

CONTRIBUTING.md

@@ -1,11 +1,97 @@
-# Contributing code
+# Contributing Code
 
-Sorry, the 2.4 branch is still not quite ready for code contributions yet! Stay
-tuned.
+## Process
 
-Bug fixes and updates for the built-in story formats are being accepted for the 2.3 branch. Please open a PR against the `2.3-maintenance` branch.
+In general, the _last_ step in the process of contributing code is to open a PR
+on this repository. The reason why is that writing code is time-consuming, and
+it's better to get agreement on the implementation approach early instead of
+having to make considerable revisions.
 
-# Contributing localizations
+## Bugfixes
+
+Please first open an issue on this repo and check the box in the issue form
+indicating that you would like to work on a fix. We'll need to come to agreement
+on how to fix the issue in discussion on that issue. For minor or obvious bugs,
+this discussion should be very straightforward.
+
+Once agreement has been reached, a PR can be opened; please mention the issue
+number in the PR's description. It will be assigned to a GitHub project for the
+release it will be targeted to, so you can track the progress of a PR toward a
+finished release.
+
+## New Features and Enhancements
+
+Please read [Twine's design goals] first.
+
+If you think your idea meshes with these goals, open an issue on this repo and
+check the box in the issue form indicating you would like to work on a fix. We
+will need to discuss your idea in detail and come to agreement on how it will
+work. This process will likely require you to provide mock screenshots and
+explain how users will use the feature in detail.
+
+Once we have come to agreement on the UI and implementation approach, a PR can
+be opened. Please mention the issue number in the PR's description. As with
+bugfixes, PRs are assigned to GitHub projects to track releases.
+
+## 2.3
+
+PRs are no longer being accepted for the 2.3 release branch. Members of the
+community who would like to continue to update 2.3 are welcome to do so in a
+forked version of the app. (If you decide to do this, please use a different
+name than Twine for the app.)
+
+## PR Practices
+
+PRs should always:
+
+- Target the `develop` branch, not `main`.
+- Include sufficient unit test coverage. You can see a test coverage report by
+  running `npm run test:coverage`. A good guideline to deciding whether test
+  coverage is enough is to ask yourself, "Could this feature be re-implemented
+  solely by looking at the unit tests?"
+  - Unit tests for UI components should cover their behavior. Appearance does
+    not need to be unit tested.
+  - Unit tests for UI components should always include a baseline accessibility
+    test using [jest-axe].
+  - Use test components like `<FakeStateProvider>` to test resulting state
+    instead of mocking store dispatches.
+  - Put tests in a `__tests__` directory, mocks in `__mocks__`.
+- Include updates to the documentation if a feature is changing.
+- Pass `npm run lint` checks with no warnings or errors.
+- Use [prettier] for code formatting. There is a `prettier.config.js` file that
+  does a little configuration of the tool.
+
+## Code Practices
+
+- All data changes should take place through stores defined under `src/store`.
+  Components should manage as little internal state as possible.
+- Conversely, code under `src/store` should never touch UI code directly. The
+  only way these two should interact is by a component dispatching actions in
+  the relevant store.
+- Components under `src/components` should always take values and objects as
+  props, as opposed to IDs that they look up in a store. Code in `src/dialogs`
+  or `src/routes` may interact with stores.
+  - ✅ `<PassageCard passage={{id: 'abc', name: 'Untitled Passage 1', text: 'lorem ipsum'}} />`
+  - ❌ `<PassageCard passageId={123} />`
+- Components unders `src/components` should be [controlled] unless absolutely
+  necessary. This applies to all types of components, not just form fields.
+   - ✅ `<TextField value="lorem ipsum" onChange={setValue}>`
+   - ❌ `<TextField onChange={setValue}>`
+   - ✅ `<PassageCard passage={passage} onDelete={handleDelete} onMove={handleMove} />`
+   - ❌ `<PassageCard passage={passage} />`
+- Every React component should be assigned a top-level CSS class that is the
+  React component name in kebab case. All related CSS rules should use this
+  class name for scoping. This ensures that components will not overwrite each
+  other's styles.
+- Moving business logic out of React components and into `src/util` is almost
+  always a good idea.
+- Use CSS variables defined in `src/styles` as much as possible.
+- Use external libraries instead of reinventing the wheel if possible.
+- Add external type definitions to `src/externals.d.ts` as a last resort.
+  Modules that come with type definitions, or that have DefinitelyTyped types,
+  are strongly preferred.
+
+# Contributing Localizations
 
 Twine's localization strings are stored in [i18next] JSON format. There are a
 number of dedicated editors for this format, or you can just use a plain text
@@ -16,14 +102,22 @@ To add a new localization or edit an existing one:
 1. Clone the application source code using Git.
 2. Create a new branch for your work.
 3. Create or edit the appropriate file in `public/locales`. The file should be
-   named after the language code you are localizing for. (Check [the registry](lang-code-registry) to find the appropriate code).
+   named after the language code you are localizing for. (Check [the
+   registry](lang-code-registry) to find the appropriate code).
 4. If you are creating a new localization, copy the existing `en-US.json` file
    and replace the English strings there with localized ones in the new file.
 5. Commit your changes and create a pull request in GitHub. You should target
    the `develop` branch with your pull request.
 
 Once your PR has been accepted, please join the Twine internationalization
-listserv by sending an email to `[email protected]`. This is a low-traffic listserv that will be used to notify people who have worked on localization on Twine when future versions require localization work, e.g. when new text is added to the application.
+listserv by sending an email to `[email protected]`. This is
+a low-traffic listserv that will be used to notify people who have worked on
+localization on Twine when future versions require localization work, e.g. when
+new text is added to the application.
 
+[jest-axe]: https://www.npmjs.com/package/jest-axe
 [i18next]: https://www.i18next.com/
-[lang-code-registry]: https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry
+[lang-code-registry]: https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry
+[prettier]: https://prettier.io
+[controlled]: https://reactjs.org/docs/forms.html#controlled-components
+[Twine's design goals]: DESIGN_GOALS.md

DESIGN_GOALS.md

@@ -0,0 +1,119 @@
+# Design Goals
+
+This documents the design goals (and non-goals) Twine has. They're intended to
+guide discussion around feature suggestions and future development. Twine, like
+any piece of software, isn't perfect, and so it may not entirely live up to the
+goals stated here.
+
+Each of the goals has a set of bullet points that discuss their implications in
+practice, but they shouldn't be considered a complete list.
+
+## Easy to Learn
+
+It takes around 5-10 minutes to explain how to use Twine to make a story with
+basic links. This simplicity is key to Twine's success. A core part of Twine's
+audience are people who have had no previous programming experience and may not
+even be particularly knowledgable about computers.
+
+- Twine avoids features that are complicated to explain to a new user. These
+  features are often better-suited to tools aimed at more advanced users, like
+  [extwee] and [tweego]. Twine doesn't exclude use by advanced users, but it
+  prioritizes beginners.
+- Twine prefers providing users with sensible defaults that can be changed later
+  instead of blocking actions and asking for decisions. For example, creating a
+  new passage creates it with a placeholder name instead of showing a modal
+  dialog asking the user to provide a name, when the user may not know what they
+  want to call it yet.
+- Twine avoids using [modes], allowing users to work on multiple tasks in
+  parallel, or to start a task and come back to it later.
+- Actions in Twine are undoable, allowing users to easily reverse mistakes.
+- Twine's user interface explains itself to users, and strives to be
+  discoverable.
+  - It re-uses interface patterns users are likely to already be familiar with
+    when possible.
+  - It includes explanatory links like "What is a story format?"
+  - It avoids technical jargon.
+  - Features are not placed in contextual menus or available through keyboard
+    shortcuts only, where only a user who has read documentation might know about
+    them.
+  - It uses icon-only buttons extremely sparingly. The intent of a button
+    labeled with text is almost always clearer than one that only has an icon.
+- [The Twine Reference][twine-ref] comprehensively covers Twine's features.
+  (It's not a tutorial or how-to guide to writing with Twine, though--other
+  community resources serve that purpose.)
+
+## Accessible
+
+There are several dimensions in which Twine strives to be accessible. The
+dimensions listed here are equally important.
+
+Twine is accessible to users with disabilities.
+
+- User interface development is guided by [WCAG guidelines]. In particular:
+  - Twine is usable for users who use screen readers like JAWS, NVDA, or
+    VoiceOver.
+  - As much of Twine as possible is usable by a someone who only uses a
+    keyboard. Many users only use a keyboard or assistive technology that
+    emulates a keyboard.
+- Twine is covered by unit tests that check for basic accessibility problems.
+  These unit tests are not comprehensive but serve as a baseline.
+
+Twine is accessible to users regardless of the language they speak.
+
+- No language or locale receives preferential treatment by Twine.
+  - Twine tries to detect the language the user's computer is set to, instead of
+    defaulting to US English on first startup.
+- All language in Twine is localized.
+- Twine properly handles input for users who use right-to-left languages.
+
+Twine is accessible to users who interact with their computer with touchscreen
+input, as well as those who use mouse and keyboard.
+
+- Interaction targets like buttons and text fields are sized and spaced so that
+  they can be used comfortably on a touchscreen.
+- Interactions triggered by pointing a cursor at something in Twine are avoided,
+  because these don't have an equivalent on most touchscreens. If they do exist,
+  alternatives that are usable on touchscreens also exist.
+
+## Web Native
+
+Twine's home is the web. Although many dedicated Twine users use it in its
+desktop app version (abbreivated here as "app Twine"), there is a significant
+population of users who use the online version (abbreviated "browser Twine").
+
+It's difficult to estimate an exact number of browser Twine users because, out
+of respect for users' privacy, Twine does not include tracking like Google
+Analytics. But in one month in 2022, there were more than 100,000 requests for
+browser Twine at https://twinery.org/2 in server logs. (This number excludes
+known crawlers like Google, as well as requests for all of the assets that the
+online editor loads.) A request is more-or-less a single editing session in
+browser Twine, so most likely a single person using Twine in a month will
+generate multiple requests. Regardless, the point here is that a significant
+number of users regularly use browser Twine.
+
+Staying web native has also meant that adapting Twine for multiple platforms has
+been relatively easy thanks to projects like [Electron], and it ensures that
+Twine will be usable for years if not decades to come.
+
+- App Twine and browser Twine users have, as much as possible, identical
+  experiences.
+- App Twine users have identical experiences regardless of what operating system
+  they use.
+- Browser Twine supports as many modern browsers as is feasible, and users of
+  browser Twine should have identical experiences regardless of which browser
+  they use.
+  - The major exception is Safari, which has imposed [restrictions on local
+    storage](safari-localstorage) which are admirable in their goal of
+    protecting user privacy, but have dire implications for Twine users, who can
+    easily lose all of their work if they aren't careful. If it becomes possible
+    to use browser Twine in Safari safely, it would be worthwhile to make this happen.
+- The load time--which loosely equates to the download size--of Twine matters
+  and new dependencies should be carefully considered before being adopted.
+
+[extwee]: https://github.com/videlais/extwee
+[tweego]: https://www.motoslave.net/tweego/
+[modes]: https://en.wikipedia.org/wiki/Mode_(user_interface)
+[wcag guidelines]: https://www.w3.org/WAI/standards-guidelines/wcag/
+[twine-ref]: https://twinery.org/reference/en/
+[electron]: https://www.electronjs.org
+[safari-localstorage]: https://webkit.org/blog/10218/full-third-party-cookie-blocking-and-more/

README.md

@@ -11,10 +11,10 @@ This is a port of Twine to a browser and Electron app. See
 The story formats in minified format under `story-formats/` exist in separate
 repositories:
 
--   [Harlowe](https://bitbucket.org/_L_/harlowe)
+-   [Harlowe](https://foss.heptapod.net/games/harlowe/)
 -   [Paperthin](https://github.com/klembot/paperthin)
 -   [Snowman](https://github.com/klembot/snowman)
--   [SugarCube](https://bitbucket.org/tmedwards/sugarcube)
+-   [SugarCube](https://github.com/tmedwards/sugarcube-2)
 
 ### INSTALL