fix: memo copy link broken and memo refreshing never ending #261
+33
−44
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.
Hi, I was testing the app after a while of not using it and noticed right away that refreshing the memo list by swiping down in the main screen didn't ever finish, so I looked into the code to find out the cause.
It seems like the implementation for copying links for memos relied on collecting updates of the
currentAccount
variable at the AccountService to find out the host for the current account. However, using.collect{}
with DataStore.data (the underlaying structurecurrentAccount
gets its value from) may never finish, as it suspends for every update in the store, impeding theloadMemos()
function in the view model from finishing, by trying to load the host each time the memo list was loaded.I saw the recommended way was to use
.first()
withrunBlocking{}
if needed, but thought using a StateFlow tied to the lifecycle of the view model may be better to keep the latest value loaded (though it shouldn't change unless the current account was switched). This way I also avoided deeply passing the host to each card, and instead reading it from the view model (to avoid injecting the AccountService to each card). Not sure if this is the best decision though, but as the list view model was already referenced and frequently used in each card, thought it would be better this way.I also noticed that the link generated was formed with the format
${host}/m/${identifier}
which was invalid from what I tested with the latest version v0.24.4 of memos. Therefore, I changed it to just use${host}/${memo.identifier}
, after reading the API v1 docs. I'm not sure if I may be breaking compatibility with the v0 API, but the docs I could find of that weren't clear enough about the format of the identifiers, and any mention I could find of it was accompanied by a strong encouragement to use API v1, so please let me know if I need to make any changes.