Skip to content

Conversation

@jmattheis
Copy link
Member

@jmattheis jmattheis commented Aug 4, 2025

Closes #696
Fixes #695
Fixes #574
Fixes #762

@jmattheis jmattheis requested a review from a team as a code owner August 4, 2025 10:11
endif
DOCKER_BUILD_IMAGE=gotify/build
DOCKER_WORKDIR=/proj
DOCKER_RUN=docker run --rm -e LD_FLAGS="$$LD_FLAGS" -v "$$PWD/.:${DOCKER_WORKDIR}" -v "`go env GOPATH`/pkg/mod/.:/go/pkg/mod:ro" -w ${DOCKER_WORKDIR}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eternal-flame-AD Do you have time to give this PR a small manual test? It should work as the e2e tests are successful, but there may be something I overlooked.

The UI coloring changed a little because the defaults of the material-ui lib changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do today or tomorrow!

Previously, expanded messages where collapsed again when scrolling some
messages further. The saved height in the virtual list wasn't changed.
This caused the list to flicker when scrolling to the previously
expanded message again.
@codecov
Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.54%. Comparing base (a9ecbdc) to head (d99b423).
⚠️ Report is 33 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #818      +/-   ##
==========================================
- Coverage   79.55%   79.54%   -0.01%     
==========================================
  Files          56       56              
  Lines        2646     2645       -1     
==========================================
- Hits         2105     2104       -1     
  Misses        450      450              
  Partials       91       91              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
Copy link
Member

@eternal-flame-AD eternal-flame-AD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can try some service worker stuff so latest messages always displays regardless of Internet connection, similar to the Android App.

@eternal-flame-AD
Copy link
Member

eternal-flame-AD commented Aug 5, 2025

Here's a demo script that creates a variety of messages under realistic scenarios (and they are all free assets so we can use it for branding).

It would also be nice if we can get clickURL to work on web frontend.

https://gist.github.com/eternal-flame-AD/de79b3431281c802548d9c0b4dbbdc47

screenshot

eternal-flame-AD and others added 2 commits August 5, 2025 19:26
Copy link
Member

@eternal-flame-AD eternal-flame-AD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I don't see any regressions so good to merge. It's a long wanted feature so I am good with just get it out of the door first and then we can see maybe new features (the service worker and more interactive extras support)

Maybe update the screenshot because the UI now looks different.

SHELL := /bin/bash
ifdef GOTOOLCHAIN
GO_VERSION=$(GOTOOLCHAIN)
else
Copy link
Member Author

@jmattheis jmattheis Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconnecting doesn't correctly refresh the currently visible data.

reconnect.webm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how you made this happen, I blocked /message and clicked refresh and can get stuck in this state.

Probably it's MessagesStore#loadMore needs a .finally(() => (this.loading = false)), I think it is an old bug bc IIRC I accidentally got it stuck refreshing before too.

Copy link
Member Author

@jmattheis jmattheis Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a separate problem, I'll fix this too. The one I can reproduce happens after restarting gotify/server. The cause of this bug is that the state is cleared after a reconnect happens (there could be new messages while the websocket was disconnected, so we do a full refresh). This happens here

reaction(
() => stores.currentUser.connectionErrorMessage,
(connectionErrorMessage) => {
if (!connectionErrorMessage) {
clearAll();
loadAll();
}
}
);
the pages like Messages currently do the initial fetch of the data via a useEffect.
React.useEffect(() => {
if (!messagesStore.loaded(appId)) {
messagesStore.loadMore(appId);
}
}, [appId]);

This useEffect isn't reexecuted after the state is cleared by the reaction. On the current master version this is done in a different way that is reexecuted.

@jmattheis jmattheis merged commit ebf6a64 into master Aug 8, 2025
4 checks passed
@jmattheis jmattheis deleted the upgrade-ui branch August 9, 2025 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Update the remark-gfm dep Migrate react-scripts to vite Error: error:0308010C:digital envelope routines::unsupported

3 participants