We are generally looking to move away from hooks as they are hard to
reason about and have weird bugs. In particular, the timer did not
properly capture the ref of the callback, and so it would execute on old
state. A standard stateful widget does not have this problem, and is
easier to organise.
Co-authored-by: Alex <alex.tran1502@gmail.com>
The widget is not recreated correctly when videoPlayName changes, which
can cause weird behaviour with hooks and timers (like timers not firing
when they should do). Using a key forces flutter to recreate the widget
properly and allow the assumptions in build to work correctly.
* fix(mobile): don't update search filters in-place
Search filters are currently modified in-place, which can feel quite
janky. The chips behind the bottom sheet update instantly, and the
search page gets confused because filters have been applied but no
search has been initiated. Filters should keep their own copy of the
filter when they're opened, and the commit + search on apply.
The previous filter and pre-filter concepts were also cleaned up. They
added complexity, and `search()` now owns the full life cycle of the
filter.
This now also reverts the changes from #27155, as this solution should
be cleaner.
* refactor & color tweak
---------
Co-authored-by: Alex <alex.tran1502@gmail.com>
3 seconds is too long for some people, and it can be confusing to see
the video not playing and no indication that it's buffering. Reducing
the duration of the timer should show the spinner faster and prevent
confusion.
Co-authored-by: Alex <alex.tran1502@gmail.com>
fix(mobile): reset video controls hide timer when showing controls changes
The hide timer currently only resets when the status of the video
changes, but does not account for when the controls change. This means
that two things happen:
1. The hide timer does not reset when the controls become visible
again, the controls will stay visible forever or until the playback
status changes.
2. The hide timer will fire too quickly, and will hide the controls
much sooner than 5 seconds if the controls are hidden and then shown
again before the hide timer fires.
At current, the controls for videos are always hidden when opening an
asset from the timeline, and when swiping between assets. The latter is
actually quite annoying, so it would be better UX if video controls were
hidden when opening from the timeline like before, but visibility of the
controls was retained when swiping between assets.
These toasts can sometimes cover UI elements and make them impossible to
interact with until they are dismissed. Specifically, deleting an asset
will show a toast over the video controls and prevent seeking.
The spacing was required for the old slider, but the new one has its own
spacing and makes it redundant. There is too much now, and we've
received feedback that it should be less sparse. The default track
height of 16px is an improvement over the old track height, but it is
very thick. A middleground of 12px might be better.
Ensure that all files are flushed after they've been written.
At current, files are not explicitly flushed to disk, which can cause
data corruption. In extreme circumstances, it's possible that uploaded
files may not ever be persisted at all.
* feat(mobile): use material design 3 slider
The new slider is easier to use, and looks more modern.
* chore: add shadow to button and text for better visibility
---------
Co-authored-by: Alex <alex.tran1502@gmail.com>
The numbers in the backup page are not monospace, and so changes cause
the layout to shift. Using tabular figures (monospace) will prevent
that.
Refs: #25021
Videos have recently been changed to support zooming, but this can make
the controls in the centre of the screen unergonomic as they will either
stay in the centre when dismissing, or stick to the video when zooming.
Neither is great. We should align the behaviour with other apps which
has the play/pause toggle at the bottom of the screen with the seeker
bar instead.
Co-authored-by: Alex <alex.tran1502@gmail.com>
* fix(mobile): correct local asset dimensions
We are constraining the size of videos so that they play nicely with
hero animations, and don't stretch in weird ways. This however caused a
regression as we are not account for local assets on Android which have
un-oriented dimensions.
* post-orientation width and height in local sync
* migration
* no need to handle it in asset viewer
---------
Co-authored-by: mertalev <101130780+mertalev@users.noreply.github.com>
Co-authored-by: Alex <alex.tran1502@gmail.com>
Search results use a different provider than the main timeline, and they
appear appear to have diverged a bit. This means that assets can
sometimes look wrong or different in search compared to the main
timeline or albums.
The background of the photo view does not extend below the height of the
viewport, and so the asset details fade in over black with the photo
view, and the standard surface colour scheme of the scaffold for the
rest. This leads to a janky animation. We can't change the background of
the scaffold to black, as it in turn makes the iOS bouncing scroll
physics cut off incorrectly. The best fix is to remove background
decoration from the photo view, and defer to the parent to colour the
background.
Co-authored-by: Alex <alex.tran1502@gmail.com>
Search results are replaced with a spinner when loading the next page,
which is quite jarring. Search results now remain visible when loading
the next page with a spinner at the bottom. The next page also loads
sooner, which makes it feel a lot smoother.
Co-authored-by: Alex <alex.tran1502@gmail.com>
In order to scroll smoothly without interfering with the gesture
detector on the photo view, we have an offstate scroll view which we
defer all drags to, and then forward scroll offsets to the real scroll
controller. This works well, but it can be simpler. Instead, we can
create a custom scroll controller on a scroll view with never scrollable
physics, and then forward drag events to that, bypassing the need for a
proxy scroll controller.
Co-authored-by: Alex <alex.tran1502@gmail.com>
Consolidate video state into a single asset-scoped provider, and reduce
dependency on global state generally. Overall this should fix a few
timing issues and race conditions with videos specifically, and make
future changes in this area easier.
The image in the photo view has no height, and is therefore entirely
unconstrained. This causes the image to take up the full height of the
viewport during the hero animation, which can make look out of sync. In
some other cases, it can stretch or resize the image to fill the entire
viewport.
The changes in #25952 inadvertently removed an optimisation which
prevents the video player from being recreated when the tree changed.
This happens surprisingly often, namely when the hero animation
finishes. The widget is particularly expensive, so recreating it 2-3 in
a short period not only feels sluggish, but also causes the video to
hitch and restart.
The solution is to bring the global key back for the native video
player. Unlike before, we are using a custom global key which compares
the values of hero tags directly. This means we don't need to maintain a
map of hero tags to global keys in the state, and also means we don't
have to pass the global key down multiple layers.
This also fixes#25981.
Asset details are prematurely hidden when a drag ends if the simulation
shows that it will close given its current velocity. It makes for a much
more responsible feeling UI. However, this behaviour conflicts with the
logic which determines whether details are showing based on the current
offset. The result is that the details are hidden, then immediately
shown again, and then hidden once it passes the min snap distance
threshold.
This can be fixed by only evaluating the position based logic when a
drag is active, and then inferring upcoming state with a simulation.
Keeping track of the last scroll offset and guarding on scroll direction
is not necessary. The dead zone with kTouchSlop is more than sufficient,
and much simpler.
Co-authored-by: shenlong <139912620+shenlong-tanwen@users.noreply.github.com>
We have all the information we need to decide on whether we should pop
or not at the end of a drag. There's no need to track that separately,
and update the value constantly.
We were manually tracking whether gestures should be blocked, which was
a remnant of how the old code worked. This is no longer needed as we
have better heuristics for knowing whether we should skip drag updates
now.
Co-authored-by: Alex <alex.tran1502@gmail.com>
The drag intent was not set until it reached the kTouchSlop threshold.
This is not necessary as flutter already has its own heuristics for
preventing unintended drags.
The result of using kTouchSlop is that dismissing or scroll can feel a
little delayed, and will jump from 0 to kTouchSlop (18px) rather than
moving smoothly.
The existing implementation for showing asset details uses a bottom
sheet, and is not in sync with the preview or scroll intent. Other apps
use inline details, which is much cleaner and feels better to use.
Some widgets, like Icon widgets, automatically inherit opacity from the
icon theme in the context. Many other widgets however, do not. The
Immich logo, profile picture, and backup badge are examples of widgets
of this.
All unsupported toolbar widgets have been updated to support inheriting
the opacity from the icon theme.
IconButtons internally animate properties like opacity, which is kind of
nice, but means we have to do more work to replicate that behaviour for
other widgets. In most cases, we can simply use an IconButton widget and
forward the correct opacity. The Immich logo however is not a button,
and therefore we need to use a custom TweenAnimationBuilder.
All widgets are using efficient, native opacity rather than the heavy
Opacity widget.
* feat(mobile): dynamic multi-line album name
Album names are currently limited to a single line, and scroll on
overflow. It would be better if album names were multi-line, and even
better if the font size was dynamic depending on how many lines there
are. The album name should then overflow with an ellipsis.
This is actually quite similar to how Google Photos handles album names.
* lint
---------
Co-authored-by: timonrieger <mail@timonrieger.de>
The latest version is already hidden in the server info widget if
disabled (https://github.com/immich-app/immich/pull/25691), however I
did not realise there are more places where this warning is shown. This
hides the warning everywhere, and cleans up the code a bit.
The server storage info has a lot of whitespace due to the ListTile.
Converting it to be inline makes the styling appear more intentional.
There are also a few semantically relevant list items in the app bar
dialog which have been grouped together.
I'm testing changes to animations and app performance, and noticed it
felt quite sluggish on a 120hz display. It turns out that high refresh
is disabled in debug builds. It's probably a good idea to enable it so
that it more closely mirrors the production build.
The current asset changes when the timeline refreshes, which can be
quite jarring. Assets are tracked by their index, and that index becomes
stale when the timeline refreshes. This can be resolved by updating the
index of asset based on a unique identifier (like the hero tag).
* fix(mobile): hide latest version if disabled
If the version check feature is disabled, the server will currently send
stale data to the client. In addition to no longer sending stale data,
the client should also not show the latest version if the feature is
disabled.
This complements the server PR #25688.
* lint
---------
Co-authored-by: Alex <alex.tran1502@gmail.com>
The current behaviour will intersect if the page is scrolled about 90% down
which works okay for a small number of assets, but does not scale well with
large amounts of assets. Instead of relying in proportional values, it may be
more sensible to use a consistent measure for loading more pages. A simple and
sensible suggestion may be to load another page when there is only one more
viewport worth of assets to display. It can be refined and revisited in future,
but it seems to work relatively well in my testing and prevents the issues
which occur with large amounts of assets.
Co-authored-by: Alex <alex.tran1502@gmail.com>
The height of the search results element was unrestricted, which meant that the
asset visibility calculations were completely incorrect. The consequence of
this is that assets which should not have been visible, were. In practical
terms, all assets below the viewport were rendered when they shouldn't have
been which is terrible for performance. Limiting the height of the viewport
fixes that calculation and assets are correctly hidden.
The consequence of limiting the height of the viewport is that the intersector
then incorrectly thought the scroll position was always at the end. This has
been fixed by calculating the position of sliding window against the calculated
asset layout container height.
Co-authored-by: Alex <alex.tran1502@gmail.com>
The thumbhash had a z-index setting which meant it would cover the search bar,
and would always cause weird animations when scrolling up in search results.
This is fixable by removing the z-index and moving it in front the other
elements to get a naturally higher higher z-index preference.
The contents of search results are slightly offset by the search bar, search
terms and spacing (margins/padding), and needs to be factored in when
calculating whether an asset is visible or not. The offset was 0, which
meant that assets were removed from view too early.
The timeline has been quite aggressive with scrolling to assets, even if they
were right in the middle of the page. If the asset is visible, then we
shouldn't scroll to it. It's really confusing when assets jump around after
being viewed.
The search results page can become unstable with large amounts of assets, and
has therefore been limited to displaying just 5000 assets. This limit is
arbitrary and may be too restrictive.
We would like to move away from the concept of finding and removing pending
jobs. The only place this is used is for album update notifications, and this
is done so that users who initially uploaded assets to an album will also
receive a notification if someone else then adds assets to the same album. This
can also be achieved with a job for each recipient. Multiple jobs also has the
advantage that it will scale better for albums with many users, it's possible
to send notifications concurrently, retries are possible without sending
duplicate notifications, and it's clear what recipient a job failed for.
* chore(server): don't insert embeddings if the model has changed
We're moving away from the heuristic of waiting for queues to complete. The job
which inserts embeddings can simply check if the model has changed before
inserting, rather than attempting to lock the queue.
* more robust dim size update
* use check constraint
* index command cleanup
* add create statement
* update medium test, create appropriate extension
* new line
* set dimension size when running on all assets
* why does it want braces smh
* take 2
---------
Co-authored-by: mertalev <101130780+mertalev@users.noreply.github.com>
Job workers are currently started on app init, which means they are started
before the DB is initialised. This can be problematic if jobs which need to use
the DB start running before it's ready. It also means that swapping out the
queue implementation for something which uses the DB won't work.
The class 'overflow-y-scroll' will always show the scrollbar. Changing
this to 'overflow-y-auto' will only show the scrollbar when needed.
All instances of the 'overflow-y-scroll' class have been changed.
Fixes: #7190
included all thumbnail metadata. It seems this has to be explicitly disabled.
Refs: #4382
feat. basic metadata e2e test
fix: use tiff thumbnails in first step + e2e fix
fix: revert switch to tiff
feat: test metadata of both webp and jpg
feat: use upload in e2e test
fix: lint
strip metadata with exiftool
use `withIccProfile`
fix e2e
formatting
run jobs in e2e
The dependency on rxjs has been removed in favour of iterators as it's clearer
and the nature of the workload is inherently non-reactive. The uncaught error
when the list of files is empty has also been implicitly fixed by this change.
Fixes: #3300
The omission of additional cache-control directives implied the resource could
be stored in shared/public caches, which is not desirable.
In addition, the no-transform directive will ensure content is not
unintentionally mangled.
Fixes: #3014
The NGINX gzip module does not compress responses with a status of 201, which is
a major issue specifically for the /api/asset/time-bucket endpoint where
responses can be upwards of 5Mi. The size of the response is dramatically
reduced with gzip to 500Ki in some cases.
https://trac.nginx.org/nginx/ticket/471https://trac.nginx.org/nginx/ticket/394
The signature of these endpoints should be GET rather than POST anyway, but that
is a bigger discussion.
The bars are 2 pixels tall, which can be tricky to click. Additional padding
increases the height to 16 pixels, without changing how it looks, and makes for
much easier clicking.
In addition, remove the onDestroy lifecycle for the tween as it's not
necessary. It was a relic from using animation frames.
Manifest list digests can be found with:
```sh
docker buildx imagetools inspect python:3.11.4-bullseye
docker buildx imagetools inspect python:3.11.4-slim-bullseye
docker buildx imagetools inspect ghcr.io/nginxinc/nginx-unprivileged:1.25.0-alpine3.17
```
The node images are pinned in #2736Fixes#2751
Partially fixes#2752
It look like Svelte has a concept of 'tweening' for writing animations, which should reduce the complexity of the animation code.
Thanks to @probablykasper for finding this.
A lot of the logic has been rewritten for reactivity, which further reduces
complexity.
The current implementation mixes intervals and animation frames, which is a
little convoluted. The use of intervals means that the animation is not going
to be smooth and may have strange behaviour when the window is moved to the
background. It's possible that the current animation frames could pile up and
run all at once which would be undesirable.
Moving everything into animation frames means the code is simpler and easier to
reason about. It should also be more performant and less buggy.
Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
Given #2668 introduced support for imagemagick and libraw, this should no
longer be necessary which allow for reduced code footprint and complexity.
Fixes: #2744
* use imagemagick and libraw for raw image support
imagemagick and libraw have generally good support for raw images, including
Sony's ARW format. These tools should also allow Immich to support many more
image formats in future without any major code changes.
https://www.libraw.org/supported-cameras
I've tested and verified this change with .ARW files and other standard formats.
Fixes: #2156
* Add additional type for awr
* pr feedback
---------
Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
It's perfectly valid to have an email address without a TLD, for instance:
- test@localhost
- test@svc-in-same-k8s-namespace
- test@internal-corp
Fixes#2667
This is required to support raw images as Alpine 3.18 included fixes to imagemagick.
Related: #2156
In addtion, the images have stricter tags and are pinned with a digest. The
manifest list digest can be found using:
```sh
❯ docker buildx imagetools inspect node:18.16.0-alpine3.18
```
A default entrypoint and command make it just a bit easier to use the images as
there is no longer a need for an explicit entrypoint. The exception is the
server image, which still requires the shell script to be specified.
