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

Update translations from Weblate #2

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

github-actions[bot]
Copy link

Automated changes by create-pull-request GitHub action

E-m-i-n-e-n-c-e and others added 30 commits February 7, 2025 13:33
Fixed a bug in MessageListTheme.lerp() where dmRecipientHeaderBg was using
streamMessageBgDefault instead of dmRecipientHeaderBg for interpolation.
I forgot to do this before today's v0.0.26, oops.

The process is only semi-automated at this point (see zulip#276).  It
was fresher in mind as of the last couple of releases, but I didn't
think of it today.  That's what a checklist is for; add it there.
This update followed a more boring normal process again, the same as
c9c5f3d.  Unlike 833b5fd, no manual adjustment needed; the
workaround that the adjustment in that commit was part of seems to
have succeeded in getting Weblate to handle `nb` appropriately.
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The string is used at the end of the "errorFilesTooLarge" message,
which includes a list of files with its size that are too large.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The message, when used in lib/widgets/compose_box.dart, substitutes
`listMessage` with newline separated lines of filenames with size.
Update the example to match this usage.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
to match the labels for the other fields (loginEmailLabel,
loginPasswordLabel, etc.)

This also updates existing translations in other languages to
match.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This avoids a potential race if the queried account is logged out
between the invocation of this Builder callback and
`MaterialApp.onGenerateInitialRoutes` (if such a race is possible).
And remove the use of Builder widget, which is unncessary after
this refactor.
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
This fixes a potential bug, in case the server returned
`realm_url` contains a trailing `/`.
I ran into this today when I tried running the Linux app.

(It looks like the reason I hadn't seen it before is that until
recently I had this package installed; it was installed
"automatically", i.e. only because it was a dependency of
something else, and then I upgraded my machine, it was no longer
such a dependency, and got autoremoved.  Now that I've installed it
directly, it'll stay.)
This allows us to call it from model code when GlobalStore is available.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is more about testing the implementation of PerAccountStoreWidget
to handle routeToRemoveOnLogout, instead of logOutAccount.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
plus moving and refactoring the tests to match

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
`PerAccountStore` shouldn't be an owner of the `MessageListView`
objects.

Its relationship to `MessageListView` is similar to that of
`AutocompleteViewManager` to `MentionAutocompleteView` (zulip#645).

With two owners, `MessageListView` can be disposed twice when
`removeAccount` is called (when the user logs out, for example):

1. Before the frame is rendered, after removing the `PerAccountStore`
   from `GlobalStore`, `removeAccount` disposes the `PerAccountStore` ,
   which disposes the `MessageListView` (via `MessageStoreImpl`).

   `removeAccount` also notifies the listeners of `GlobalStore`.

   At this point `_MessageListState` is not yet disposed.

2. Being dependent on `GlobalStore`, `PerAccountStoreWidget` is rebuilt.
   This time, the StatefulElement corresponding to the `MessageList`
   widget, is no longer in the element tree because `PerAccountStoreWidget`
   cannot find the account and builds a placeholder instead.

3. During finalization, `_MessageListState` tries to dispose the
   `MessageListView`, and fails to do so.

We couldn't've kept `MessageStoreImpl.dispose` with an assertion
`_messageListView.isEmpty`, because `PerAccountStore` is disposed before
`_MessageListState.dispose` (and similarily
`_MessageListState.onNewStore`) is called.  Fixing that will be a future
follow-up to this, as noted in the TODO comment.

See discussion:
  https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/MessageListView.20lifecycle/near/2086893

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Just so there's no confusion about whether this is true again
this year.

(The substance of the project description lives at the linked page,
in the zulip/zulip repo, and already got updated last week.)
This is the class we actually use at this point -- not
StickyHeaderListView -- so it's good for it to have some docs too.
This way the example can be used to demonstrate the next cluster
of bug fixes working correctly, before yet fixing the next issue
after those.
gnprice and others added 19 commits February 13, 2025 23:33
This is nearly NFC.  The sense in which it isn't is that if any of
the `tester.drag` steps touched a header -- which listens for tap
gestures -- then this would ensure that step got interpreted as a
drag.  The old version would (a) interpret it as a tap, not a drag,
if it was less than 20px in length; (b) leave those first 20px out
of the effective length of the drag.

The use of DragStartBehavior.down fixes (b).  Then there are some
drags of 5px in these tests, subject to (a); TouchSlop fixes those
(by reducing the touch slop to 1px, instead of 20px).

This change will be needed in order to have the item widgets start
recording taps too, like the header widgets do, without messing up
the drags in these tests.
This will be useful in a test we'll add for an upcoming change.
These changes are NFC for the existing double-sliver example,
with the sticky header at the top.
The existing `.first` and `.last` versions rely on the order that
children appear in the Flutter element tree.

As is, that works because the `_Item` widgets are all children of one
list sliver, and it manages its children in a nice straightforward
order.  But when we add multiple list slivers, the order will depend
also on the order those appear as children of the viewport; and in
particular the items at the edges of the viewport will no longer
always be the first or last items in the tree.  So introduce a couple
of custom finders to keep finding the first or last items in the
sense we mean.

For examples of using the APIs these finders use, compare the
implementation of [FinderBase.first].
This will be helpful for keeping complexity down when we add more
slivers to this list.
This is still enough to fill more than the viewport, and will be more
helpful for scrolling past an entire sliver when we add more slivers.
There are still some bugs affecting the sticky_header library when a
SliverStickyHeaderList occupies only part of the viewport (which is
a configuration we'll need for letting the message list grow in both
directions, for zulip#82).

I sent a PR which aimed to fix a cluster of those, in which I tried
to get away without writing these systematic test cases for them.
It worked for the cases I did test -- including the cases that would
actually arise for the Zulip message list -- and I believed the
changes were correct when I sent the PR.  But that version was still
conceptually confused, as evidenced by the fact that it turned out
to break other cases:
  zulip#1316 (comment)

So that seems like a sign that this really should get systematic
all-cases tests.

Some of these new test cases don't yet work properly, because they
exercise the aforementioned bugs.

The "header overflowing sliver" skip condition will be removed later
in this series.

The "paint order" skips will be addressed in an upcoming PR.
…xtent

This fixes a latent bug: this method would give wrong answers if the
sliver's paintExtent differed from its layoutExtent.

The bug is latent because performLayout currently always produces a
layoutExtent equal to paintExtent.  But we'll start making them differ
soon, as part of making hit-testing work correctly when a sticky
header is painted by one sliver but needs to encroach on the layout
area of another sliver.  The framework calls this method as part of
hit-testing, so that requires fixing this bug too.
This makes for fewer situations to think about at a given point in
the code, and will make the logic a bit easier to follow when we
make some corrections to the overflow case.
This commit is NFC for the actual app, or at least nearly so.

This call to calculatePaintOffset was conceptually wrong: it's
asking how much of this sliver's region to be painted is within the
range of scroll offsets from zero to headerExtent.  That'd be a
pertinent question if we were locating something in that range of
scroll offsets... but that range is not at all where the header
goes, unless by happenstance.  So the value returned is meaningless.

One reason this buggy line has survived is that the bug is largely
latent -- we can remove it entirely, as in this commit, and get
exactly the same behavior except in odd circumstances.
Specifically:
 * This paintedHeaderSize variable can only have any effect
   by being greater than childExtent.
 * In this case childExtent is smaller than headerExtent, too.
 * The main way that childExtent can be so small is if
   remainingPaintExtent, which constrains it, is equally small.
 * But calculatePaintOffset constrains its result, aka
   paintedHeaderSize, to at most remainingPaintExtent too,
   so then paintedHeaderSize still won't exceed childExtent.

I say "main way" because the alternative is for the child to run out
of content before finding as much as headerExtent of content to
show.  That could happen if the list just has less than that much
content; but that means the header's own item is smaller than the
header, which is a case that sticky_header doesn't really support
well anyway and we don't have in the app.  Otherwise, this would
have to mean that some of the content was scrolled out of the
viewport and then the child ran out of content before filling its
allotted remainingPaintExtent of the viewport (and indeed before
even reaching a headerExtent amount of content).  This is actually
not quite impossible, if the scrollable permits overscroll... but
making it happen would require piling edge case upon edge case.

Anyway, this call never made sense, so remove it.

The resulting code in this headerExtent > childExtent case still
isn't right.  Removing this wrong logic helps clear the ground for
fixing that.
This case has several bugs in it.  Not coincidentally, it's tricky to
think through: there are several sub-cases and variables involved
(the growth direction, vs. the header-placement direction, vs. the
coordinate direction, ...).  And in fact my original PR revision
which fixed the cases that would affect the Zulip message list
was still conceptually confused, as evidenced by the fact that it
turned out to break other cases:
  zulip#1316 (comment)

One step in sorting that out was the preceding commit which split
this overflows-sliver case from the alternative.  As a next step,
let's expand on the reasoning here a bit, with named variables
and comments.  In doing so, it becomes more apparent that several
points in this calculation are wrong; for this NFC commit, mark
those with TODO-comments.  We'll fix them shortly.
When the sticky header overflows the sliver that provides it -- that
is, when the sliver boundary is scrolled to within the area the
header covers -- the existing code already got the right visual
result, painting the header at its full size.

But it didn't work properly for hit-testing: trying to tap the
header in the portion where it's overflowing wouldn't work, and
would instead go through to whatever's underneath (like the top of
the next sliver).  That's because the geometry it was reporting from
this `performLayout` method didn't reflect the geometry it would
actually paint in the `paint` method.  When hit-testing, that
reported geometry gets interpreted by the framework code before
calling this render object's other methods.

Specifically, this sliver was reporting to its parent that its
paint region (described by `paintOrigin` and `paintExtent`) was
the same as the child sliver's paint region.  In reality, this
sliver in this case will paint a larger region than that.

Fix by accurately reporting this sliver's paint region (via
`paintOrigin` and `paintExtent`), while adjusting `headerOffset`
to be relative to the new truthful paint region rather than the
old inaccurate one.

This fix lets us mark a swath of test cases as no longer skipped.

On the other hand, this change introduces a different bug in this
overflow case: the child sliver is now painted in the wrong place if
_headerAtCoordinateEnd().  (It gets lined up with the inner edge of
the header, even though it's too short to reach the viewport edge
from there.)  That bug is latent as far as the Zulip app is concerned,
so we leave it for now with a TODO.

Other than that, after this fix, sticky headers overflowing into
the next sliver seem to work completely correctly... as long as
the viewport paints the slivers in the necessary order.  We'll
take care of that in an upcoming PR.
Prepares for zulip#1130, this commit is almost NFC with only
difference that, in an error case we previously emitted a
`ParagraphNode` containing an `UnimplementedInlineContentNode`
(along with any adjacent nodes), now we emit a single
`UnimplementedBlockContentNode` instead.
@github-actions github-actions bot force-pushed the update-translations/weblate branch from 55f30ff to 2cd3f29 Compare February 17, 2025 10:09
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.

6 participants