Update translations from Weblate #1
Open
Conversation
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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This reverts part of commit a9b2016 (zulip#958), keeping the `Zulip.xcconfig` file and its references. The fix was released in version `3.6.0` of `package:firebase_core` and version `15.1.3` of `package:firebase_messaging`: firebase/flutterfire@d7d2d4b We have since then updated Firebase dependencies 3 times: zulip@24215f6 zulip@1abb0a0 zulip@3ba9985
Changelog: https://pub.dev/packages/pigeon/changelog#2500 There's one breaking change which removes an option we don't use, so it shouldn't affect us. This commit is the result of running these commands: flutter pub upgrade --major-versions pigeon ./tools/check pigeon --all-files --fix
The `fromVersion` reference inside this test body, for each of the test cases, was referring to the same one, mutable `fromVersion` variable. So by the time any of them ran, its value was 5, and they tested migrating from 5 to 2, 5 to 3, 5 to 4, 5 to 5 respectively. Meanwhile the test names reflected the intended migrations, because the names were computed eagerly, reading the value of `fromVersion` before moving on to the next iteration. To fix the issue, make a fresh `fromVersion` binding inside each iteration of the loop.
Conceptually this is really a static fact about the class, not a fact about a given instance of it. (The implementation's body is always just a constant integer.) Make things a bit clearer -- and make it easier to get hold of the value from outside the class -- by making that explicit, with a static const field as the home of the value.
And adjust a test to make that true in tests too.
As explained in the parent commit, and asserted just above the _dropAndCreateAll call site, this fact always holds. That lets us simplify how downgrades work, and as a bonus cut one step from the checklist for making schema updates. This reverts some of the changes made in 601936d.
No transactions here will do any good if the changes aren't in a transaction with the `PRAGMA user_version =` statements that update the database's record of what version the schema is at, and therefore of which migrations might be needed. And if Drift is setting up for us such a transaction, it'll necessarily enclose the whole migration anyway.
While this appears to be a user facing change, it's not visible yet, not until TopicName.displayName becomes nullable. This is a part of a series of changes to handle empty topics. Signed-off-by: Zixuan James Li <[email protected]>
While this appears to be a user facing change, it's not visible yet, not until TopicName.displayName becomes nullable. This is a part of a series of changes to handle empty topics. A test is skipped because the server does not send empty topics to the client without "empty_topic_name" client capability. Signed-off-by: Zixuan James Li <[email protected]>
This method is trivial for now, but provides a home for logic to be added next. In particular this separates the computation of how the topic should appear in the hint text from what the topic should be in the API, in `destination`.
Previously, "Message #stream > (no topic)" would appear as the hint text when the topic input is empty but mandatory. Now, it is shown as "Message #stream" instead, since the "(no topic)" isn't allowed when topics are mandatory. Co-authored-by: Zixuan James Li <[email protected]>
This does not rely on TopicName.displayName being non-nullable or "empty_topic_name" client capability, so it is not an NFC change. The key change that allows sending to empty topic is that `textNormalized` can now be actually empty, instead of being converted to "(no topic)" with `_computeTextNormalized`. --- This is accompanied with a content input hint text change, so that "Message #stream > general chat" appears appropriately when we make TopicName.displayName nullable. --- Previously, "Message #stream > (no topic)" was the hint text for content input as long as the topic input is empty and topics are not mandatory. Showing the default topic does not help create incentive for the user to pick a topic first. So only show it when they intend to leave the topic empty. See discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/general.20chat.20design.20.23F1297/near/2088870 --- This does not aim to implement hint text changes to the topic input, which is always "Topic". We will handle that as a follow-up. Signed-off-by: Zixuan James Li <[email protected]>
As we add more settings, this will keep things better clustered logically together.
As we add more features using the database and more queries we make from it here at the app's startup, this will help us check that we aren't unduly delaying startup. Results on my Pixel 8, with 2 account records: db load time 91.9ms total: 1.5ms init, 85.4ms settings, 5.0ms accounts db load time 87.8ms total: 1.7ms init, 78.1ms settings, 8.0ms accounts db load time 93.3ms total: 1.2ms init, 83.7ms settings, 8.5ms accounts db load time 78.1ms total: 1.4ms init, 71.8ms settings, 4.8ms accounts db load time 86.9ms total: 1.2ms init, 77.8ms settings, 7.9ms accounts
This aligns these tests more closely with how we expect most app code to interact with these settings (using GlobalStoreWidget.settingsOf).
This is the same abbreviation we use in implementation code. Especially when the name occurs repeatedly in a line, this can make the code usefully more concise.
…rations This new table to read looks to add something like 5-8ms to the app's startup time. Not nothing; but not a big cost compared to the time we spend loading server data right afterward. Specifically, results on my Pixel 8 (compare to those in the previous commit): db load time 117.9ms total: 15.2ms init, 89.2ms settings, 5.1ms bool-settings, 8.3ms accounts db load time 90.9ms total: 1.3ms init, 78.5ms settings, 8.3ms bool-settings, 2.8ms accounts db load time 87.2ms total: 1.4ms init, 71.8ms settings, 5.9ms bool-settings, 8.1ms accounts db load time 85.7ms total: 1.2ms init, 72.8ms settings, 4.9ms bool-settings, 6.8ms accounts db load time 91.3ms total: 1.4ms init, 80.0ms settings, 7.5ms bool-settings, 2.5ms accounts db load time 83.8ms total: 1.1ms init, 70.0ms settings, 5.0ms bool-settings, 7.7ms accounts
We'll add a new subclass for the "Cancel" / "Save" banner in the upcoming edit-message compose box.
In the Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4010-6425&m=dev https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-17028&m=dev the banner label's 9px vertical padding is factored as 5px applied to the whole banner, and 4px applied to just the label, excluding trailing buttons. Follow that, so it can work as intended when we add such buttons.
And add some features to _Banner to support that TODO, and a Figma link.
Before, the 8px end padding would just be added to any SafeArea padding. Now, it's done using SafeArea.minimum, so it doesn't get too big.
When we add a _Banner subclass for the edit-message compose box, we'd like it to control its own label instead of expecting it from callers.
We'll use this to customize the behavior. It also makes a handy marker in tests, when using the testing-only [ScrollPosition.activity] getter for inspecting what's going on in the scroll behavior.
As long as the bottom sliver is size zero (or more generally, as long as maxScrollExtent does not change during the animation), this is nearly NFC: I believe the only changes in behavior would come from differences in rounding. This change handles the case where the end turns out to be closer than it looked at the beginning of the animation. Before this change, the animation would try to scroll past the end in that case. Now it stops at exactly the end -- just like it already did in the case where the end was known exactly in advance, as it currently always is in the actual message list. That case is a possibility as soon as there's a bottom sliver with a message in it: scroll up so the message is offscreen and no longer built; then have the message edited so it becomes shorter; then scroll back down. It's impossible for the viewport to know that the bottom sliver's content has gotten taller until we actually scroll back down and cause the message's widget to get built. In order to customize this behavior, this change uses a feature I added recently (for this purpose) to DrivenScrollActivity upstream: flutter/flutter#166731
As long as the bottom sliver is size zero (or more generally, as long as maxScrollExtent does not change during the animation), this is nearly NFC: I believe the only changes in behavior would come from differences in rounding. By describing the animation in terms of velocity, rather than a duration and exact target position, this lets us smoothly handle the case where we may not know exactly what the position coordinate of the end will be. A previous commit handled the case where the end comes sooner than estimated, by promptly stopping when that happens. This commit ensures the scroll continues past the original estimate, in the case where the end comes later. That case is a possibility as soon as there's a bottom sliver with a message in it: scroll up so the message is offscreen and no longer built; then have the message edited so it becomes taller; then scroll back down. It's impossible for the viewport to know that the bottom sliver's content has gotten taller until we actually scroll back down and cause the message's widget to get built. And naturally that will become even more salient of an issue when we enable the message list to jump into the middle of a long history, so that the bottom sliver may have content that hasn't yet been scrolled to, has never been built as widgets, and may not even have yet been fetched from the server. In order to control the behavior with a Simulation rather than a fixed endpoint and duration with a Curve, this commit uses a feature I added recently for this purpose to DrivenScrollActivity upstream: flutter/flutter#166730
This makes it possible to see in a self-contained way, in this class's own code, that it always starts moving at a velocity that isn't zero, or less than zero, or at risk of being conflated with zero. This doesn't have a big effect in practice, because the only call site already does something else whenever the distance to travel is negative or very close to zero. But there is a small range -- namely where the distance to travel is between 1 and 12 physical pixels, given the default behavior of ScrollPhysics.toleranceFor -- in which this minimum speed does apply.
These tests are about scrolling, and there's no other sort of controller in sight. So just `controller` is a good name for the scroll controller.
These were implicitly assuming that the scroll position at the bottom of the list is 0.0. That's currently true, but we'll soon let it vary. Make them a bit more general in saying what they mean.
This has already gotten out of sync with the total number of children in the list, after 5c70c76 added a TypingStatusWidget child while leaving this unchanged. On the other hand, its spec isn't to be the total number of children: it's to be the total number of children "that will contribute semantic information". So the spacer shouldn't count, and probably TypingStatusWidget and MarkAsReadWidget shouldn't either when they don't show anything. (Currently it effectively counts the spacer and MarkAsReadWidget, or one could retcon that to TypingStatusWidget and MarkAsReadWidget.) Further: > If the number is unknown or unbounded this should be left unset > or set to null. So it seems like when `haveOldest` is false, this should be null. Meanwhile, the exact value here seems to have little effect. I just tried the app out in TalkBack, and there are some other issues (when you scroll around, there's some glitchy behavior with focus moving to the sticky header and back, and even with the scroll position sometimes abruptly jumping back; and I couldn't find a way to navigate by a whole message or item at a time, though that might have been my inexperience with TalkBack) but it doesn't ever quote the number of list items as far as I could tell, and isn't bothered by this being a few more or less than the spec'd number. So for the moment let's do something simple: don't count any of the extra children at the end, the ones that are often or always empty. This also makes a couple of tests a bit simpler to follow. Thanks to Zixuan for raising the question. Discussion here: zulip#1468 (comment)
Thanks to all the preparatory changes we've made in this commit series and those that came before it, this change should have only subtle effects on user-visible behavior. This therefore serves as a testing ground for all the ways that the message list's scrolling behavior can become more complicated when two (nontrivial) slivers are involved. If we find a situation where the behavior does change noticeably (beyond what's described below), that will be something to investigate and fix. In particular: * The sticky headers should behave exactly as they did before, even when the sliver boundary lies under the sticky header, thanks to several previous commit series. * On first loading a given message list, it should start out scrolled to the end, just as before. * When already scrolled to the end, the message list should stay there when a new message arrives, or a message is edited, etc., even if the (new) latest message is taller than it was. This commit adds a test to confirm that. Subtle differences include: * When scrolled up from the bottom and a new message comes in, the behavior is slightly different from before. The current exact behavior is something we probably want to change anyway, and I think the new behavior isn't particularly better or worse. * The behavior upon overscroll might be slightly different; I'm not sure. * When a new message arrives, the previously-latest message gets a fresh element in the tree, and any UI state on it is lost. This happens because it moves between one sliver-list and the other, and the `findChildIndexCallback` mechanism only works within a single sliver-list. This causes a couple of visible glitches, but they seem tolerable. This will also naturally get fixed in the next PR or two toward zulip#82: we'll start adding new messages to the bottom sliver, rather than having them push anything from the bottom to the top sliver. Known symptoms of this are: * When the user has just marked messages as read and a new message arrives while the unread markers are fading, the marker on the previously-latest message will (if it was present) abruptly vanish while the others smoothly continue fading. We have a test for the smooth fading, and it happened to test the latest message, so this commit adjusts the test to avoid triggering this issue. * If the user had a spoiler expanded on the previously-latest message, it will reset to collapsed.
This makes our first payoff in actual UX from having the message list split into two back-to-back slivers! With this change, if you open a message list and the latest message is very tall, the list starts out scrolled so that you can see the top of that latest message -- plus a bit of context above it (25% of the viewport's height). Previously the list would always start out scrolled to the end, so you'd have to scroll up in order to read even the one latest message from the beginning. In addition to a small UX improvement now, this makes a preview of behavior we'll want to have when the bottom sliver starts at the first unread message, and may have many messages after that. This new behavior is nice already with one message, if the message happens to be very tall; but it'll become critical when the bottom sliver is routinely many screenfuls tall.
…impls Thanks Greg for noticing this: zulip#1484 (comment)
github-actions
bot
force-pushed
the
update-translations/weblate
branch
from
9304b62 to
9a27a11
Compare
This removes the ad-hoc color we use for null subscriptions, which is a workaround for not having logic to generate different base colors within constraints for unsubscribed channels: https://chat.zulip.org/#narrow/channel/48-mobile/topic/All.20channels.20screen.20-.20.23F832/near/1904009 While this change does not implement that logic either, it removes the ad-hoc color in favor of a constant base color, intended for colorSwatchFor. The base color can be reused on topic-list page (zulip#1158), giving us access to `.iconOnBarBackground` and friends for the app bar.
This is for the upcoming local-echo feature, to hide the timestamp. There isn't a Figma design for messages without a timestamp.
This will make it easier to support comparing the conversations between subclasses of MessageBase. The message list tests on displayRecipient are now mostly exercising the logic on Conversation.isSameAs, which makes it reasonable to move the tests. Keep them here for now since this logic is more relevant to message lists than it is to the rest of the app. Dropped the comment on _equalIdSequences since it is now private in the short body of DmConversation.
This is NFC because MessageListMessageItem is still the only subclass of it.
This will make MessageItem compatible with other future subclasses of MessageListMessageBaseItem, in particular MessageListOutboxMessageItem, which do not need unread markers.
github-actions
bot
force-pushed
the
update-translations/weblate
branch
from
9a27a11 to
e4e1641
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Automated changes by create-pull-request GitHub action