Update translations from Weblate#2
Merged
AshutoshKhadse23 merged 60 commits intomainfrom Mar 26, 2025
Merged
Conversation
This has a natural default, at least in tests, because most tests don't care about the specific value of the global settings. (By contrast most tests that interact with the store at all will care what set of accounts exist, so there isn't as natural a default for `accounts`.) Originally noticed this here: zulip#1386 (comment)
I believe this is effectively already done by the existing assert `_perAccountStores.containsKey`, because of the invariant that _accounts contains all the keys in _perAccountStores. But the explicitness seems helpful. (And loadPerAccount relies on the account existing.)
In particular: GlobalStore.loadPerAccount GlobalStore.doLoadPerAccount UpdateMachine.load The first two already have an `assert` that the account exists, and the third has a ! null-check. They're called in a chain from two places, and we have the account in both: the initial account load and the reload (GlobalStore._reloadPerAccount).
…tId] In GlobalStore.perAccount, the loadPerAccount future can throw, and in that case the `_perAccountStoresLoading.remove` in these lines of code hasn't been getting called. That's only a latent bug, currently, because loadPerAccount only throws if the account was logged out, and we always do a `_perAccountStoresLoading.remove` on logout (in GlobalStore.removeAccount). But we'd like to add another case where the loadPerAccount future throws (disallow connecting to ancient servers), which doesn't involve logout / removeAccount. So this try/finally will be important for that case.
For zulip#1354, we'll need an alternative to TestGlobalStore that runs UpdateMachine.load with prepared API responses instead of mocking it. Since FakeApiConnection doesn't yet offer a `Completer`-based way of completing/awaiting requests, these two tests will have to diverge, preparing and awaiting different delays as needed.
Soon, we'll add an alternative to TestGlobalStore whose doLoadPerAccount makes fake API requests; its doLoadPerAccount will call UpdateMachine.load. These methods will be useful for that new class. (The new class makes sense as a sibling of TestGlobalStore, rather than a subclass, because TestGlobalStore's dartdoc says it makes no network requests.)
Thanks to the introduction of PerAccountStore.updateMachine in be6698e, this feature in the test code is no longer needed.
The ImageMagick `convert` command, introduced in b13cf4e, apparently inserts the current time into the file. That makes the build not reproducible. Fix that using the "strip-nondeterminism" tool from the Reproducible Builds project: https://reproducible-builds.org/docs/timestamps/ The resulting change to the affected file can be seen using `sng`, and purely removes a pair of timestamps: $ f=ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-1024x1024@1x.png \ && diff -U3 <(git show @:"$f" | sng) \ <(<"$f" sng) --- /dev/fd/63 2025-01-06 21:11:42.422900540 -0800 +++ /dev/fd/62 2025-01-06 21:11:42.426900539 -0800 @@ -13,14 +13,6 @@ bKGD {red: 255; green: 255; blue: 255;} pHYs {xpixels: 161260; ypixels: 161260; per: meter;} # (4096 dpi) tEXt { - keyword: "date:create"; - text: "2023-11-27T17:11:13-08:00"; -} -tEXt { - keyword: "date:modify"; - text: "2023-11-27T17:11:13-08:00"; -} -tEXt { keyword: "Software"; text: "www.inkscape.org"; }
I believe the Inkscape version I was using the last time I generated these files was Inkscape 0.92.4. The version now on my machine is Inkscape 1.2.2. In Inkscape 1.0, they made some changes to the `inkscape` CLI. The old way of specifying where to put the output is deprecated, and now produces these warnings: $ tools/generate-logos Warning: Option --export-png= is deprecated Warning: Option --export-png= is deprecated Warning: Option --export-png= is deprecated […] So, switch to the new way. (The new version also produces slightly different output; the old and new versions look identical to my eye, though examining with the `sng` tool confirms some pixels have slightly different values. Leaving those changes out for now, to bundle them with substantive changes coming later in this commit series.)
This is a better home than tools/.
This makes these SVGs a bit easier to reason about. More concretely, this will allow us to switch from Inkscape to librsvg for the conversion: the latter is faster and has some other useful properties, but doesn't support using xlink to point to a separate file as these were doing. This change is nearly NFC: it'd be NFC if the image processing were mathematically exact. In reality the old and new output images look identical to my eye, but examining with `sng` (like in the previous commit upgrading Inkscape) confirms some pixels have slightly different values. Like with that commit, we leave those changes out for now, to bundle them with substantive changes coming later in this commit series.
This SVG implementation is more convenient than Inkscape's for
several reasons:
* It's faster -- the runtime for the whole script goes from about
5.5s down to 1.5s.
* It doesn't include a redundant alpha layer (of a constant 0xff)
in the output. That makes all the PNG outputs smaller, and
better yet lets us cut out the mess of ImageMagick `convert`
we'd been using for stripping that layer in the one file where
Apple forbids it.
* Most concretely: it supports the `transform` CSS property.
That will come in very handy in the next commit, to help us
stitch together different pieces of the new icon design.
For a useful table of which SVG implementations support which
features, see this page from resvg:
https://linebender.org/resvg-test-suite/svg-support-table.html
(At the moment resvg isn't in Debian stable, though -- presumably
because Rust packaging is somewhat hard -- so resvg itself was
less convenient for me to use.)
Just like with the last couple of commits, this produces a visually
imperceptible change in the output files. Examining with `sng`
shows some pixels did change slightly, and also that some metadata
got simpler. Here's from Icon-20x20@2x.png:
IHDR {
width: 40; height: 40; bitdepth: 8;
- using color alpha;
-}
-pHYs {xpixels: 6299; ypixels: 6299; per: meter;} # (160 dpi)
-tEXt {
- keyword: "Software";
- text: "www.inkscape.org";
+ using color;
}
+bKGD {red: 255; green: 255; blue: 255;}
Others are similar but with width, height, xpixels, ypixels, and
the DPI figure all scaled up; and in Icon-1024x1024@1x.png the old
version had no "alpha" but did have sections cHRM and gAMA.
Like with a couple of previous commits in this series, we leave
those changes out for now, in order to bundle them with
substantive changes coming next.
Fixes zulip#415. Fixes zulip#1254. Fixes zulip#1401. Fixes zulip#1402. Thanks to Vlad for developing the new design through several iterations. Changes from the old version include: * The background gradient is more saturated. (zulip#1254) * The "BETA" label is cleaner, and has a more balanced layout relative to the main "Z" of the logo. (zulip#415) * The icon is "adaptive" using the API introduced in Android 8, which lets it be handled more nicely by Android launchers / home screens that want to put the app icon in a circle (like on Pixel) or an iOS-style squircle (like on Samsung devices). In particular this should avoid it getting shrunk and placed on a bed of white padding. (zulip#1402, originally filed on zulip-mobile in 2020.) * The assets include a monochrome foreground icon, which makes the app work with the "themed icons" introduced in Android 13, for users who choose to enable that. (zulip#1401, originally filed on zulip-mobile by a user report last year.) This commit continues to use "BETA" versions of the logo for the actual app, but also adds the corresponding non-beta versions of the source SVGs so that we can smoothly switch to those when it comes time to launch the app. The relevant area in Figma is around here: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6549-5962&t=mMvSGpqwbgJBIv6U-0 but the exact story of these source files is somewhat complicated, because there are a number of technical requirements and it's taken several rounds of iteration between design and engineering to get to this result. Specifically: * zulip-white-z-beta-on-transparent.svg was "beta-foreground.svg" from Vlad here: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/updated.20app.20icon/near/2072480 * zulip-white-z-on-transparent.svg and zulip-gradient.svg were from Vlad in this message (as code blocks): https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/updated.20app.20icon/near/2038233 * Then zulip-combined.svg was "app_icon_1.svg" here: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/updated.20app.20icon/near/2030417 which I produced as described in that message from "app_icon.svg" here: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/updated.20app.20icon/near/2030569 * And zulip-beta-combined.svg I assembled myself, in a text editor, out of those pieces. * I also wrote ic_launcher.xml.
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The url_launcher documentation warns that the meaning of platformDefault may change at any time. We've chosen particular behaviors we want on different platforms, so express those choices explicitly. This change is NFC given the current behavior of url_launcher, which appears to be that platformDefault means an in-app browser for at least HTTP links. Co-authored-by: Greg Price <greg@zulip.com> Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This preserves the previous behavior (external application on iOS, and in-app browser on Android) when browserPreference is `null`. There is no way for the client to update the setting for now, hence the NFC. The helper could have been a static method instead of an extension, similar to ThemeSetting.displayName, but that will be a lot more verbose for the caller. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The UI currently does not have a design, and is made to be as simple as possible to implement for now. Fixes: zulip#1228 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The streamId of the moved messages does not change as intended, and the topic remains the same; this is not a valid message move. We will later introduce a data structure that captures inconsistencies of this kind. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The fields orig_subject and propagate_mode are always present on message moves. See also API documentation: https://zulip.com/api/get-events#update_message Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is NFC except in certain malformed cases: where either newStreamId is present and equals origStreamId, or newTopic is present and equals origTopic. The API docs say neither of those should happen. This version is useful because it brings this code closer to how it will look when we move this logic of "were the messages moved, and how" into the API parsing code so that other parts of the data model can reuse the results.
…ues sooner To see that this is NFC, first observe that the only possible behavior change this could cause is to change some event from being ignored (hitting a `return` in this section of code) to being processed by the code below, or vice versa. (It might also change what particular message gets printed to the debug log; but that doesn't count as a behavior change.) Then both before and after this change, the events that this overall set of conditions accepts are those that (a) have all of origTopic, origStreamId, and propagateMode non-null, and (b) have at least one of newTopic or newStreamId with a non-null value different from origTopic or origStreamId respectively. This change is useful because it brings this logic closer to how it will look when we move it into the API parsing code. While here, we add a debug log message on unexpected propagateMode, to further bring this closer to how it will look in the API parsing code. Co-authored-by: Zixuan James Li <zixuan@zulip.com>
This is mostly NFC except that we were logging and ignoring malformed event data before, and now start to throw errors within the parsing code. This data structure encapsulates some checks so that we can make all fields non-nullable, with reasonable fallback values. As of writing, we do not use origStreamId (a.k.a.: 'stream_id') when there was no message move, even though it is present if there were content edits This makes dropping 'stream_id' when parsing `moveData` into `null` acceptable for now. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This does not correctly catch the case when `origTopic='topic'`, and `newTopic=origStreamId=newStreamId=null`. Instead, remove it and rely on the assertions from UpdateMessageMoveData's constructor. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This isn't really any longer than invoking it, and cuts out a layer of indirection.
This can be seen as another important aspect of what it means for ensureGlobalSettings to be functioning properly.
We don't currently end up using this anywhere.
This was done recently in 8cbea53 and the other commits of 1167.
This helpfully makes it impossible for these migration steps to accidentally call methods on this database instance directly, rather than on their Migrator parameters `m`. As a bonus, the code gets three levels less indented.
Like with _migrationSteps, this ensures we don't accidentally call methods on the AppDatabase instance instead of on the given Migrator.
This is a little more complicated in the code overall; but it simplifies the logic that runs each time the app starts up.
Now that this method no longer has the job of ensuring there's a record in the table, its name can reflect that simpler role, replacing "ensureGlobalSettings". This simpler role also makes one test case no longer relevant.
…edWidget This did the exact same thing as the base implementation does.
…t.settingsOf This allows widgets to listen separately for changes to settings or to the rest of the global store's data, which is a bit nice. More fundamentally, as a matter of code organization, it gives us a place to put code related to the store of global settings where there's more room to spread out than if we kept it in GlobalStore, and where the namespace is already specific to global settings. Those two points in turn will also help us adjust the API the rest of the app uses for interacting with these settings, so as to make it more ergonomic. The TODO comments added in this commit mark places where this API will be more coherent after upcoming commits in this series.
This gives _GlobalSettingsStoreInheritedWidget essentially the same structure as _GlobalStoreInheritedWidget, which hopefully makes the pair of them a bit easier to think about.
To save code that consults the settings from an extra indirection ".data", we equip GlobalSettingsStore with getters for all the fields of the data class. This also makes the store now the natural home for the additional members that had been in an extension on the data class.
It's natural that the settings found directly on the global store are the global settings.
This commit makes GlobalSettingsStore more self-contained, taking over from GlobalStore the responsibility for updating settings in the database at the same time as they get updated in the in-app cache [GlobalSettingsStore._data]. To do that, it needs to call doUpdateGlobalSettings, the abstract method that's implemented separately to use a live database in the real app vs. a vacuous fake backend in tests. But that means that method can no longer live on GlobalStore: for GlobalStore needs a GlobalSettingsStore, but then GlobalSettingsStore would need a GlobalStore, forming a cycle. In particular when calling the GlobalSettingsStore constructor from within the initializer list of the GlobalStore constructor, we can't pass `this` as an argument because `this` hasn't yet been constructed. (If we were determined to make such a cycle work, we could do so by having a late or nullable field. But let's take the cycle as a signal that the design can be cleaned up.) So, make a new class GlobalStoreBackend as the home for this abstract method. A GlobalStore now "has a" GlobalStoreBackend, rather than "is a" GlobalStoreBackend as it effectively was before. As a result we can readily construct a "backend" instance first, and pass that to the GlobalSettingsStore constructor, before going on to finish constructing the overall GlobalStore. Probably most or all of the other functionality that LiveGlobalStore implements should get moved to the new "backend" class too, but we'll leave that for possible follow-up changes.
This completes the transition to GlobalSettingsStore as the self-contained store for the global settings.
This makes the call sites significantly nicer to read. Relatedly, it turns the specifics of how the settings are represented in the database into more of an implementation detail of the settings store.
Thanks Zixuan for noticing we haven't been covering this: zulip#1410 (comment)
Flutter PR flutter/flutter#159999 (fixing a bug where MediaQuery provides an incorrect TextScaler) breaks this test; fix it with isSystemTextScaler. Co-authored-by: Chris Bobbe <cbobbe@zulip.com>
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
7254e68 to
22bb612
Compare
AshutoshKhadse23
pushed a commit
that referenced
this pull request
Dec 27, 2025
I happened to notice this message getting printed repeatedly in the
debug logs (reformatted a bit):
[ERROR:flutter/runtime/dart_vm_initializer.cc(40)] Unhandled Exception:
NetworkException: HTTP request failed. Client is already closed.
(ClientException: HTTP request failed. Client is already closed.,
uri=https://chat.zulip.org/api/v1/users/me/presence)
#0 ApiConnection.send (package:zulip/api/core.dart:175)
<asynchronous suspension>
#1 Presence._maybePingAndRecordResponse (package:zulip/model/presence.dart:93)
<asynchronous suspension>
#2 Presence._poll (package:zulip/model/presence.dart:121)
<asynchronous suspension>
That'd be a symptom of an old Presence continuing to run its polling
loop after the ApiConnection has been closed, which happens when the
PerAccountStore is disposed. Looks like when we introduced Presence
in 5d43df2 (zulip#1619), we forgot to call its `dispose` method.
Fix that now.
The presence model doesn't currently have any tests. So rather than
try to add a test for just this, we'll leave it as something to
include when we write those tests, zulip#1620.
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
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