Editing the caption of a media message dropped all of its formatting: bold,
links and the rest came back as plain text, and saving wrote that plain text
over the formatted caption.
The composer was being opened on the plain caption. Editing a message body
already starts from its HTML when rich text editing is on, and the caption
path was the only one that did not - it read `captionOrNull()` and never had
the caption's HTML to read, because the timeline content only kept the
already-parsed `formattedCaption`.
Media contents now carry the caption's HTML alongside the parsed one, the
same way text contents carry `htmlBody`, and editing a caption starts from it
when rich text editing is on. The save path already sent the composer's HTML,
so nothing changes there.
Part of #6730
Deleting a message that headed a thread took the thread with it: the "N
replies" summary under the message disappeared and there was no way back to
the replies, which are still there.
Every branch of the timeline content mapper passes `MsgLikeContent`'s thread
information on except the redacted one, which returned a bare
`RedactedContent` and dropped it. The thread summary lives on
`MsgLikeContent` next to `kind`, so it survives the redaction; only the app
was throwing it away. `RedactedContent` now carries the thread info like the
other contents do, and `EventTimelineItem.threadInfo()` reads it.
A deleted message that still heads a thread is also left out of the "N
removed messages" group, which would otherwise hide the summary again as soon
as three deletions ended up next to each other.
Part of #7397
Every @BindingContainer now lives in a di subpackage, in a file named after
it. Containers that were declared inside a shared file (SimplePlayer.kt,
LockScreenConfig.kt, VoiceMessagePresenter.kt, TimelineItemPresenterFactories.kt,
MediaItemPresenterFactories.kt) are extracted into their own file.
Method names are made consistent too:
- @Binds functions are named bind<ConcreteParameterType>, fixing three that
named something else (notably RoomAliasResolverBindingContainer, whose
provider was still called providesJoinRoomPresenterFactory).
- @Provides functions are named after what they provide, and all use the
provides prefix rather than a mix of provide and provides.
ANDROID_KEYSTORE moves back to the cryptography impl package, next to
KeyStoreSecretKeyRepository which uses it; it is not DI specific and would
otherwise be unreachable from there.
No behaviour change: Metro discovers containers via @ContributesTo and does
not care about method names, so the only call site to follow was in
DefaultVoiceRecorderTest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Metro's @BindingContainer replaced Dagger's @Module, but the declarations
kept the old Module suffix. Rename them (and their dedicated files) so the
type name matches the annotation, matching UserStatusBindingContainer which
already followed this convention.
No behaviour change: Metro discovers containers via @ContributesTo, so there
were only two call sites to update, both in tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 174 commits develop gained since this branch was cut touch preview data
in three ways, and each is resolved so the flipped konsist rule still holds
across the merged tree.
Two screens the branch had renamed were deleted upstream by "Remove the
account provider search screen and homeserver resolver": the
changeaccountprovider and searchaccountprovider packages. Their deletion
wins, so ChangeAccountProviderStatePreviewParam, SearchAccountProviderStatePreviewParam
and the two views that used them go with them rather than coming back
under the new name.
Two preview classes develop added still carried the old suffix and are
renamed here, along with the `@PreviewParameter` reference in each view:
GlobalSearchStateProvider -> GlobalSearchStatePreviewParam and
SendFailureDialogStateProvider -> SendFailureDialogStatePreviewParam.
Everything else merged textually. No production class extending
PreviewParameterProvider is left with a `Provider` name, and no reference
to any of the 194 renamed names survives outside the CHANGES.md entry that
records a past pull request title.
* Model the fake voice recorder on a cancellable recording job
FakeVoiceRecorder pushed a whole recording synchronously from a single
suspending call, which made it both weaker and stricter than
DefaultVoiceRecorder.
Every level emission carried the same elapsed time and the emissions were
coalesced by Molecule, so no intermediate recording state was observable and
the level-limit test had to skip half the emissions to compensate. Each level
now advances the test time source and suspends for its share of the recording
duration, so the states arrive one at a time and that magic number is gone.
Call verification moved to the constructor lambdas the rest of the repo uses,
in place of hand-rolled counters and assertCalls. The counters discarded the
argument, so nothing pinned that cancelling a recording stops it with
cancelled = true; that is now asserted. Truth is no longer needed by the
fixture module.
A second startRecord() no longer raises an error where production only warns
and returns, and stopping or deleting terminates an in-flight emission loop
the way cancelling the real recording job does, so a stale recording state can
no longer arrive after the recording has finished.
* Keep each recording's level loop to its own recording
The level loop checked a single isRecording flag, which cannot tell "this
recording was cancelled" from "a later recording has started". After a cancel
followed by a new start, the first loop resumed, saw the flag set again, and
advanced the shared test time source, so the new recording reported the elapsed
time of both. Each recording now carries an id and its loop stops as soon as
that id is no longer the active one.
* Give the room name in the timeline top bar a 48dp tap target
The row carrying the room avatar and name is the control that opens the
room details, but it is only as tall as its content, which is under the
48dp minimum recommended for a touch target. Constrain it to a minimum
height of 48dp. The children stay centred inside the app bar, so nothing
moves visually.
* Update screenshots
The accessibility snapshot of the timeline is the only one the change
affects: the node which opens the room details now reports a 48dp tall
bounding box. The room name, the avatar and the icons stay where they
were.
Codecov reported 55.55% patch coverage on the previous commit: four of the nine
measured lines were only partially covered. No test paginates forwards in a
thread timeline, and the focus states Requested, Loading and Failure were never
live when a forward pagination arrived, so only the Success branch of
isPending() was ever evaluated both ways.
Add a test per uncovered state: a forward pagination is deferred while the focus
request is still being debounced and while the focused timeline is being
created, it is performed again once the request has failed, and it is dropped in
a thread timeline while a backward one still goes through.
The pagination recorder now also sits on the live timeline, and the presenter is
built on that same timeline. Before the detached timeline exists, a pagination
that escaped the gate would go to the live one, so without this the new guards
would pass against the unfixed presenter. The deferred cases end on the emitted
state rather than on cancelAndIgnoreRemainingEvents for the same reason: the
recorder throws from inside the presenter coroutine and cancelling swallows it.
Verified by reverting the gate: the three deferred-pagination tests then fail
with "This lambda should never be called."
* Refresh the room members when a mention is started
The mention suggestions read room.membersStateFlow, which is a cache filled
only by an explicit updateMembers() call. Nothing calls it while a room is
open, so a member who joined after the room was opened never appears in the
suggestion list until the room is left and entered again.
Call updateMembers() when a mention search starts. The trigger is
distinctUntilChanged on "the current suggestion is a mention", so it fires
once per mention rather than on every keystroke, and fetchRoomMembers already
no-ops while a fetch is in flight.
The `Provider` suffix reads as a dependency provider rather than as preview
data, and it collides with the real providers in the codebase — `StringProvider`,
`PickerProvider`, the login feature's `AccountProvider` — so the name says
nothing about which of the two a class is. `PreviewParam` says it.
195 classes are renamed, along with the 185 files named after them, the konsist
rule that enforces the convention, the presentation-class file templates and the
onboarding documentation.
This cannot be split. `Konsist.scopeFromProduction()` scans the whole project, so
flipping the assertion before every class has moved leaves the rule red, and a
rule that accepted either suffix during a transition would be exactly the kind of
compatibility layer this convention change is meant to remove.
Two judgement calls in the rule itself. The `name.endsWith("PreviewProvider")`
exclusion is dropped: its only job was to reject a redundant `Preview` before
`Provider`, and the new suffix already contains it; no class currently ends with
`PreviewProvider`, so removing it changes nothing today. And
`EditableAvatarViewUriProvider` is renamed rather than deleted from the exclusion
list even though no such class exists in this repository, since it presumably
comes from the modules overlaid for the enterprise build.
Nothing keyed off the old suffix: no snapshot filename contains a provider class
name, so no Paparazzi image changes, and the Kover variants match on `*Presenter`,
`*State` and `*ViewKt`, so coverage bounds are unaffected.
Opening a room from a notification asks the timeline to focus the notified
event. When that event is outside the loaded live window the controller
creates a detached timeline for it, and a detached timeline always carries a
forward loading indicator as its last chronological item. The items list is
reversed for display, so that indicator is the bottom-most row and is composed
on first layout, which unconditionally emits a forward pagination request
nobody asked for. Because a notified event sits close to the live end, that one
page reaches the end, and the controller responds by closing the detached
timeline. The live window replaces the item list while it is still anchored at
the bottom, so the user lands on the newest message and the focus request is
discarded.
Defer automatic forward pagination while a focus request is still pending. Once
the focused event has been rendered the viewport has moved off the indicator
row, so only a real scroll back to the live end switches to the live timeline,
which is what the controller's existing behaviour is for. Backward pagination
and the explicit jump-to-live action are untouched.
A m.key.verification.request sent from a client that supports in-chat
verification carries a plain text body for clients that do not. Element X
has no msgtype of its own for it, so it fell through to OtherMessageType
and the timeline printed that body, telling the user their client does
not support in-chat key verification right after a verification they had
just completed.
Drop the event when building the timeline items, as Element X iOS does.
The timeline action sheet offered Forward on a live location share, but
forwarding one always failed: Mode.Live maps to the SDK's
MsgLikeKind.LiveLocation, which RoomContentForwarder rejects outright. So
the action was shown for exactly the set of events on which it could
never work.
canBeForwarded() now returns true for a location event only when its mode
is Static. Ended live shares keep Mode.Live and fail for the same reason,
so they are excluded too. ActionListPresenter needs no change: its check
is the function's only call site in the repo, so the single gate covers
the timeline sheet, the threaded timeline and the pinned messages list.
Hiding the action mirrors Element Web and the sticker exclusion already
present in the same function. Converting a live share into a static
m.location on forward would be a product decision and is out of scope.
Relates to https://github.com/element-hq/element-x-android/issues/7190
* Expand the emoji picker to full height when the keyboard is shown
The picker content is rendered with fillMaxSize, so the sheet measures itself to
the full window height. That makes the partially expanded anchor available, and
the modal sheet auto-shows at that anchor, half way down the screen.
The sheet reserves the IME inset through its content window insets, but applies
that padding at its own bottom edge, which is off-screen while it rests half way
down. The band that is both on-screen and above the keyboard is then only about
a tenth of the screen, which is why the matching emoji end up behind it.
Expand the sheet while the keyboard is visible so its bottom edge returns to the
screen edge and the inset it already reserves lands on top of the keyboard.
Modifier.imePadding on the sheet content is not an option: the sheet applies the
insets with windowInsetsPadding, which consumes them, so a nested imePadding
reads zero.
* Cover the keyboard driven sheet expansion with a test
The IME reaction moves into ExpandSheetWhenImeIsVisible so a Robolectric test can
drive it: dispatching window insets to the composition flips
WindowInsets.isImeVisible, and the sheet state is observable from the test. Both
the shown and the hidden case are asserted, which is the patch coverage this
change was missing.
* Do not leave a blank strip below the expanded emoji search
The 10dp of bottom padding on the search bar separates the collapsed
field from the category tabs underneath it. It stayed applied once the
search bar expanded to fill the picker, so the picker stopped 10dp short
of its own bottom edge.
That strip used to sit below the visible part of a half height sheet.
Now that the sheet expands against the keyboard it reads as a gap
between the results and the keyboard, so only pad while collapsed.
Restore the snapshot backed set the store started with and declare
allowedEventIds as SnapshotStateSet so the interface says out loud that
reads of it are observable.
TimelineProtectionPresenter reads the set inside derivedStateOf, so a
plain set silently stops the timeline from re-rendering when an event is
revealed. Naming the type keeps that requirement from being mistaken for
an implementation detail.
hayaksi1
Benoit Marty
Benoit Marty
bxdxnn
Benoit Marty
Jorge Martin Espinosa
Benoit Marty
ganfra
ganfra
bmarty