Show marked/edited markers, without swipe gesture

[PIG208]

This is still a WIP.

The swipe gesture is implemented such that the marker is

• expanded when there is horizontal movement to the right
• stretched following the touch movement
• retracted when the touch is released

Notably, the content box still re-wraps as the marker pushes it to the right when it expands.To fix this, the content box might need to have a fixed size so the text can go off-screen and remain unchanged. This turned out to be tricky to accomplish.

This is stacked on top of #750. It fixes #616 and fixes part of #171.

Screen_Recording_20240624_164417.mp4

[gnprice]

Thanks! Looking forward to having this.

@terpimost @alya for feedback on how the swipe gesture behaves from a user perspective.

Notably, the content box still re-wraps as the marker pushes it to the right when it expands.To fix this, the content box might need to have a fixed size so the text can go off-screen and remain unchanged. This turned out to be tricky to accomplish.

For this I think the key is, instead of a Row, to use something like a SlideTransition (which is what Dismissible uses). If the coordinate system that one uses feels inconvenient, then PositionedTransition might be cleaner. (And we can discuss in more detail in a chat thread.)

[alya]

Pushing the context so that part of it gets hidden, rather than rewrapping, makes sense to me.

[PIG208]

The text re-wrapping issue has been fixed. I have also added a responsive fix to the star alignment issue. It's a draft because I still need to add tests for it.

[gnprice]

Great.

I was using the app this afternoon and happened to have a build from this PR on my phone, and found I really appreciated having the new information about which messages had been edited.

That caused me to think of an idea for how we can split this into a part that delivers the key functionality and that we can merge soon, and a more complex part that can come after it as a follow-up:

• For a first version, we could show just the icons, without the swipe-to-reveal feature.

  That cuts out the gesture handling, the animation, and all the tricky bits about coordinating how the marker's layout and colors change in synchrony with the motion.

  The part that remains will be exactly how we intend the final feature to look in steady state, whenever the user isn't actively pulling on one of these messages to reveal the marker. So it'll also be a good way for us to focus on polishing that design to exactly how we want it, before introducing the complexity of the swipe animation.

  From a user perspective, the one thing that's missing in this version is that it may not be super discoverable what the icons mean. But once you figure it out (perhaps by seeing them on a thread where you have other clues), you know; there's only two of them to learn.

• Then a subsequent PR introduces swipe-to-reveal and the colored markers.

I see the UI logic in this revision looks a lot cleaner, so that follow-up might happen soon after. Even then, though, I think the split will be helpful for reducing the per-PR complexity for review. So let's try making that split.

[PIG208]

I extracted a diff that implements the animation to #765. This PR should be ready for review after I have added the tests.

[PIG208]

Rebased and added some test cases for the UI change. The tests are kind of similar to those for the star, because the edit marker are star both appear under the same Stack widget.

Because of how editState is computed from json, and that toJson does not preserve it, we can't initialize the message with the edit marker present. We could implement it if needed.

[chrisbobbe]

GitHub is telling me that some conflicts have appeared; would you resolve those please?

[PIG208]

Just rebased the PR.

[PIG208]

Latest screenshots (with the offset)

Moved	Edited

[PIG208]

Latest screenshots (with the 16x16 icons and without those offsets):

Moved	Edited

[PIG208]

With the new icons, not sure if the vertical offsets are needed anymore.
This should be ready for review.

[gnprice]

Hmm, here's a screenshot of what I see:

So visually the bottom of the icon is higher than the baseline of the text, and the top of the icon is much higher than the ascender line of the text.

That's on my usual display settings, which have the font size set to small (as well as the "display size", which effectively just means my screen has more logical pixels). On default settings, I see:

So the same thing, but a little less so, because the icons aren't as much taller than the text as they are at my usual settings.

[gnprice]

On our screen-sharing call today, when you were running the app locally on Linux, it looked like you were seeing the symptom of #735. That suggests that your system has the text scaling set to >1x, i.e. the fonts are scaled up. If I crank the text size on my phone a couple of notches above default, I reproduce a layout more like your screenshots above.

When making screenshots for a piece of layout that involves text, it'd be good to do so with a 1x text scale. It can be helpful to try smaller or larger scaling too, but 1x should be the baseline.

I'm not sure what platform setting might be governing the text scaling Flutter picks up from the platform on Linux. If you can't readily find that setting, you can always add a hack locally in lib/app.dart that forces the scaling to 1x — for example a MediaQuery around the ZulipApp.

[gnprice]

For the vertical alignment for these icons, there are probably fancy things we can do to get it just right for a range of situations — but let's skip that for the present, while we move on to other issues from our tracker.

For now, let's just try to get it so that at 1x text scaling, when the message starts with a plain paragraph, the top of the edit icon doesn't stick out above the ascender line of the text, or anyway not by much more than the bottom of the icon sticks out below the baseline of the text. If we do that by a fixed few px of vertical adjustment, that'll be fine.

[gnprice]

Other than that layout issue, the code here all looks good modulo this small comment above: #762 (comment)

Thanks for the revision!

[PIG208]

Updated the PR to add the vertical adjustments and replace the outdated icon font. Thanks for noticing that scaling issue!

With the offset, default text scaling (fd38fb8)

Moved	Edited

[gnprice]

Great, thanks! Looks good — merging.

I added a comment about those vertical offsets:

+        // These offsets are chosen ad hoc, but give a good vertical alignment
+        // of the icons with the first line of the message, when the message
+        // begins with a paragraph, at default text scaling.  See:
+        //   https://github.com/zulip/zulip-flutter/pull/762#issuecomment-2232041922
         offset = const Offset(0, 2);

Also edited the PR description so that #171 doesn't get accidentally closed.