| Commit message (Collapse) | Author | Age |
|---|
| ... | |
| | |/
| |
| |
| | |
I've opted to run with `--coverage` to ensure that we continue exercising coverage support. Hat tip to @wlonk for holding me accountable on this - I had thought coverage was broken, but I was holding it wrong.
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Calling through `npm` wasn't adding anything other than complexity, and it made it somewhat harder to follow what tools did what.
I'm also pretty sure `tools/build-ui` was totally unused.
This was originally part of [another proposal][pr-6]. I've broken it out to make the intent clearer, and to make the proposal easier to get a handle on in isolation from other, related changes. Thanks to @wlonk for their input on this!
[pr-6]: https://codeberg.org/ojacobson/pilcrow/pulls/6
Merges prop/build-without-npm into main.
|
| | |/
| |
| |
| |
| |
| | |
Calling through `npm` wasn't adding anything other than complexity, and it made it somewhat harder to follow what tools did what.
I'm also pretty sure `tools/build-ui` was totally unused.
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This has a couple of material consequences:
* It will be (much) easier to reorganize the source tree, as the path to the output is no longer relative to where the config files are when building the final binary. If we do decide to move `ui` into its own child crate, we won't have to make a bunch of (very similar) changes to the Svelte build process at that time.
* There is less chance of a stale build contaminating a new one, since changes to the crate change the project hash in `OUT_DIR`. For example, while working on this change, `OUT_DIR` was at various points:
* `target/debug/build/pilcrow-7cfeef3536ddd3e7/out`
* `target/debug/build/pilcrow-09d4ddbc12bef36b/out`
* `target/release/build/pilcrow-070d373bd5f850a1`
This may use more space on disk, but it's all reclaimable with `cargo clean` and Rust is _far_ more profligate with disk space than Svelte will ever be.
* It's more consistent with Cargo's expectations around generated source files, and thus potentially easier to onboard Rust developers into.
Merges prop/ui-build-to-cargo-output into main.
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This has a couple of material consequences:
* It will be (much) easier to reorganize the source tree, as the path to the output is no longer relative to where the config files are when building the final binary. If we do decide to move `ui` into its own child crate, we won't have to make a bunch of (very similar) changes to the Svelte build process at that time.
* There is less chance of a stale build contaminating a new one, since changes to the crate change the project hash in `OUT_DIR`. For example, while working on this change, `OUT_DIR` was at various points:
* `target/debug/build/pilcrow-7cfeef3536ddd3e7/out`
* `target/debug/build/pilcrow-09d4ddbc12bef36b/out`
* `target/release/build/pilcrow-070d373bd5f850a1`
This may use more space on disk, but it's all reclaimable with `cargo clean` and Rust is _far_ more profligate with disk space than Svelte will ever be.
* It's more consistent with Cargo's expectations around generated source files, and thus potentially easier to onboard Rust developers into.
|
| |\ \
| | |
| | |
| | |
| | |
| | | |
from prop/merge-template into main
Reviewed-on: https://codeberg.org/ojacobson/pilcrow/pulls/14
|
| | |/
| |
| |
| |
| |
| |
| |
| | |
For a merge with a single incoming commit, this will generally be redundant - the default pull request subject and body are drawn from the commit itself, so they'll get repeated in the merge commit. However, for a pull request with multiple commits, the pull request title and body are likely a better merge commit message than the default offered by Codeberg.
I've been doing basically this for manual merges regardless.
Further reading: <https://forgejo.org/docs/latest/user/merge-message-templates/>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This allows developers (hi) to run tools installed via NPM without having to run `npm exec` or `npx` to do so. It's mostly for ergonomics, but it's nice to be able to run `vite build` and have it "just" work.
The "canonical" invocations, used in tool scripts or when `build.rs` has to run Node, uses `npx`. More generally, we don't assume direnv, even though the source tree provides it.
There is no corresponding `layout rust` in `direnv`, but the same niche is fulfilled by the `PATH_add` targeting Cargo's output directory. I also considered using `PATH_add` instead of `layout` for this, as they do the same thing (at least as of this version of `direnv`), but I think it's probably more appropriate to use the parts of direnv that express the broader intention to use Node's tools than to manually implement them myself.
If you're using `direnv` across this change, you'll need to re-run `direnv allow`.
|
| |/
|
|
|
|
| |
This was a simple omission on my part. In f26dd0d662d8fc33108d072031329e707f54300b, I added a `.envrc` file that was intended to be tracked, and had it reference `.envrc.local` so that others could customize the environment, through direnv, without requiring that those changes be submitted upstream.
I forgor to mark that file as ignored. It never bit us, but I tried to use a .envrc.local file today to experiment on some Node-specific PATH ideas (I want to be able to run `vite` without `npx`!) and noticed that Git wanted to track my config.
|
| |\
| |
| |
| | |
Notably, one of them was hiding a real (if unreachable) bug, by converting a "the token you have presented is not valid because the user was deleted" scenario into an internal server error, when it should have been an authorization error.
|
| | |
| |
| |
| | |
Notably, one of them was hiding a real (if unreachable) bug, by converting a "the token you have presented is not valid" scenario into an internal server error.
|
| |\ \
| |/
|/|
| |
| |
| |
| |
| | |
Tests were inadvertently broken in 96d363fd9290d43d2e6a11e2e5269fb8ccf6d65d (probably in 9f0b5b00f7ada4c5735230d0f93e04a3c58d4d7a).
The tests found a potentially-real accessibility problem! Great success.
Unfortunately, the switch to an editable div also broke the tests completely. I've marked them as skipped rather than removing them, out of optimism at the underlying bug being fixed one day.
|
| | |
| |
| |
| | |
We … can't test this, I think, because of a bug in `user-event`. Maybe there's an alternative that directly manipulates the DOM, but I'd prefer not to do that.
|
| |/
|
|
| |
The hidden `textarea` used to attach the form value to the DOM was being included in the ARIA accessibility tree, at least in testing (I didn't check in a browser). While we could suppress this iwth `aria-role="hidden"`, the WHATWG recommendation is to Not Do That, and to find another way to hide the element, instead. Marking the element as hidden accomplishes that goal, _and_ gets rid of a style rule.
|
| |\ |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In ae93188f0f4f36086622636ba9ae4810cbd1f8c9, `remote.channels.all` became a flat array of channels, instead of a map, in order to simplify some of the reasoning around how state changes propagate. However, I neglected to remove all Map-shaped calls referring to it.
This lead to some pretty interesting behaviour:
* The client could not track unread state, because reconciling local state against the remote state would find no remote state, then throw away local state entirely as a result.
* The client would not actually update when a new channel appeared.
* The client would not actually update when a channel disappeared.
|
| |\| |
|
| | |
| |
| |
| |
| |
| | |
vanished.
This may happen if the user has a link to a channel open when the channel is deleted/expires, or if they return to the app after the last channel they looked at has expired.
|
| | |
| |
| |
| | |
`session`.
|
| | |
| |
| |
| | |
channel's creation time.
|
| | |
| |
| |
| | |
been read.
|
| | | |
|
| | |
| |
| |
| | |
To make unread handling of empty channels coherent (and to make it possible to mark an empty channel as having been read), they need to be associated with a specific point in time. This change exposes their creation time in the snapshot - it was already part of the event view, though the client doesn't know that yet.
|
| |\ \
| |/
|/|
| |
| |
| | |
> We can hand-write markdown for now, as per discussions.
>
> If we have buttons and shortcuts, we'd like them to insert actual markdown into the text stream, and then, as a separate concern, we'd like to render the markdown without changing the text stream (à la Discord). But we're doing none of that now, and it's too high a piece of fruit to pluck today.
|
| | |\
| | |
| | |
| | |
| | |
| | |
| | | |
prop/text-input-no-quill-plus into prop/text-input-no-quill
Reviewed-on: https://codeberg.org/ojacobson/pilcrow/pulls/2
Reviewed-by: Kit <wlonk@noreply.codeberg.org>
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | |
| | | |
modification.
This also avoids using `placeholder` on elements where it's nonstandard, like `<div>`s.
|
| | | |
| | |
| | |
| | |
| | | |
* Give the input `div` a marker to tell screen readers &c that it is a textbox.
* Ensure that it participates in tab order. (Zero is a sentinel value, see <https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/tabindex>.)
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This is purely an aesthetic choice:
* The DOM `reset()` function can be used to clear the form, but can't be used to clear editable DIVs. Binding the editable div to a (hidden) form field allows `reset()` to clear both.
* We can find the target `form` element out of the event, but the API needed to do so differs between events dispatched to form controls and events dispatched to random DOM nodes. Using `closest('form')` works for both kinds of event target.
In practice, there is little need to make sure the message input form uses "normal" DOM APIs for functional reasons. Everything inside `MessageInput` is controllable through the component's script. This change isn't based on a functional need, but rather in the hopes that integrating with the DOM APIs makes it easier for _code we don't control_ - screen readers, password managers, saved-form support in browsers, &c - to integrate with Pilcrow. It is purely speculative.
(This also used to be necessary because Firefox didn't support `contenteditable="plaintext-only"`, but [support was added in March][ff-pto]
[ff-pto]: https://www.mozilla.org/en-US/firefox/136.0/releasenotes/#:~:text=The%20value%20plaintext%2Donly%20can%20now%20be%20specified%20for%20the%20contenteditable%20attribute%2C%20making%20the%20raw%20text%20of%20an%20element%20editable%20but%20without%20supporting%20rich%20text%20formatting.
|
| | | |
| | |
| | |
| | |
| | | |
* Suppress input (including paste) while the input is disabled.
* Style the input to make it visible that it's not accepting input.
|
| | |/
| |
| |
| |
| |
| | |
It's not much, but it makes it a bit easier to see that the placeholder text _is_ a placeholder.
Not sure what to do about it vanishing permanently once the element is edited, until the element is formally `reset()`, though.
|
| | |
| |
| |
| | |
For whatever reason, `innerText` captures interior line breaks, while `textContent` does not. Wild. DOM APIs.
|
| | | |
|
| |\ \
| |/
|/| |
|
| | |
| |
| |
| | |
This styling suggestion c/o Jessamyn Smith! Thanks, Jess. I like it.
|
| | |
| |
| |
| | |
This prevents the need to go and un-apply unwanted styles when considering code blocks.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
For reasons known only to the author, marked emits code blocks as
<pre><code>your code here</code></pre>
Inline code, on the other hand, is emitted as
<p>Non-code text <code>code text</code> non-code text.</p>
In d15bfb2b9a4872cba99bc966fe5c9c4399b3323c, we added a rule to give inline code nicer leading and trailing space, so that the borders don't directly abut the letters. However, we neglected to consider code blocks; the padding added for inline code also affected their first line, pushing it in slightly. This removes the padding from `<code>` when it is a direct child of a `<pre>`, as per the markup emitted by marked.
|
| | |
| |
| |
| |
| |
| | |
This is a fairly simple approach using a linearly-reducing scale (from 2.25 down to 1.0) to adjust both the font size and the line height. All headings are bold, and are in the body typeface.
People who actually use headings in _chat messages_ are doing a bit, but hey, bits are valid.
|
| | | |
|
| | |
| |
| |
| | |
There isn't a good way to target "any <code> not inside a <pre>" so we do a little reverse logic here, as a treat.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Our reset strips out a ton of default browser styles, which is good, but we haven't yet gotten around to adding styles we're using. This left formatted messages
feeling a
lot like
this
regardless of the intended formatting. (Is the above an example of a paragraph? A list? A single line that has gotten wrapped? The answer was "yes.")
To make the margins and padding work out nicely, I've rearranged the positioning containers used for message runs and messages. We also no longer `float` the message handles, since we no longer need to: they can be positioned relative to the message they're part of.
Styling on long bodies of inline code (`like this`) is a bit shaky. The outline overlaps with the following line. I think having a visual cue for where the code block begins and ends is _good_, but I'd like to pick apart some of the other examples on the internet because I think this needs more work.
This change also makes code blocks wrap lines at the page edge where possible (it'll still scroll if wrapping isn't possible). This won't affect _most_ code blocks much - code tends to not be that wide - and it means that using a code block for effect doesn't require people to manually wrap strings. Having tried it both ways, this feels more human.
Dumbnailing is not a _great_ solution to dealing with huge images, but it's the best we can do at rendering time. A more complete solution would require generating images at multiple sizes.
|
| |/
|
|
| |
There may be other instances of this, this is just the one I found recently.
|
| |\ |
|
| | | |
|
| | |\ |
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
There is a subtle race conditon in this code, which is likely not fixable without a protocol change:
* Ghost messages can disappear before their "real" message replacement shows up, if the client finishes sending (i.e., receives an HTTP response on the POST) before the server delivers the real message.
* Ghost messages can be duplicated briefly, if the client receives the real message before the client finishes sending.
Both happen in practice; we make no ordering guarantees between requests.
To aviod this, we'd to give clients a way to correlate pending sends with received messages. This would require fundamentally the same capabilities, like per-operation nonces, that preventing duplicate operations will require.
|
| | | |
| | |
| | |
| | |
| | |
| | | |
A `MessageRun` is a visual container with a specific layout - bordered, with a drop shadow, with a name badge on the top-left, which is either positioned to the left (`other-message`) or right (`own-message`). It is content-agnostic.
This facilitates putting things besides live messages inside of a message run. As a side effect, this gets rid of ActiveChannel; most of what it was doing makes more sense living in the channel view's `+page.svelte`.
|
| | | |
| | |
| | |
| | | |
Using a wildcard selector here makes this rule surprisingly hard to override, which will be a problem for styling unsent messages.
|
