Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix threaded mode continuesly editing same toot #2430

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Plastikmensch
Copy link

@Plastikmensch Plastikmensch commented Oct 1, 2023

Fixes #2429

Updated. See: #2430 (comment)

@Plastikmensch Plastikmensch force-pushed the fix-threaded-mode-trapped-in-edit branch from 6cb686a to eb727ff Compare October 23, 2023 02:29
@Plastikmensch
Copy link
Author

Rebased to fix a lint error which wasn't present when this PR was opened.

@ClearlyClaire
Copy link

I think I would just unset id, I'm not sure about the other behaviour, it seems pretty error-prone.

@Plastikmensch
Copy link
Author

I think I would just unset id, I'm not sure about the other behaviour, it seems pretty error-prone.

Can do, but that would mean the thread would continue on the edited toot, which is likely unexpected.

@ClearlyClaire
Copy link

Hm… I suppose it should also clear in_reply_to. After all, the UI does not let you enable threaded mode when you're editing a post, it's only possible to run into this issue if you enable it first then edit a post.

@Plastikmensch
Copy link
Author

Hm… I suppose it should also clear in_reply_to. After all, the UI does not let you enable threaded mode when you're editing a post, it's only possible to run into this issue if you enable it first then edit a post.

Wouldn't that break threaded mode?

@ClearlyClaire
Copy link

Hm… I suppose it should also clear in_reply_to. After all, the UI does not let you enable threaded mode when you're editing a post, it's only possible to run into this issue if you enable it first then edit a post.

Wouldn't that break threaded mode?

I mean, do that when id is set!

@Plastikmensch
Copy link
Author

Hm… I suppose it should also clear in_reply_to. After all, the UI does not let you enable threaded mode when you're editing a post, it's only possible to run into this issue if you enable it first then edit a post.

Wouldn't that break threaded mode?

I mean, do that when id is set!

I'm not sure I'm understanding correctly, because id is set in COMPOSE_SET_STATUS and clearing in_reply_to there would break editing replies as they wouldn't be replies anymore.

@ClearlyClaire
Copy link

id is sent when you're editing a post. I mean, basically, when a post edit is submitted, clear the compose state instead of continuing in edit mode.

@Plastikmensch
Copy link
Author

id is sent when you're editing a post. I mean, basically, when a post edit is submitted, clear the compose state instead of continuing in edit mode.

I'm still not sure what you mean, but what you're getting at is instead of continuing a thread after editing a toot in threaded mode, exit threaded mode?

@Plastikmensch
Copy link
Author

Plastikmensch commented Oct 28, 2023

Hmmh I think I get it now.
But that would mean a new thread is started after editing, as clearing the compose state explicitly doesn't exit threaded mode. The reason threaded mode exits when cancelling is because of the fallthrough.

That is why I propose saving the id of the last status in a thread, as all other solutions have undesirable side effects.
There can be an argument made whether to store the status object instead and restore the values of it after an edit instead of taking the values of the edited toot.
It pretty much comes down to what the expected behaviour should be.

@Plastikmensch
Copy link
Author

Plastikmensch commented Oct 31, 2023

Okay, made the following changes:

The second point is necessary because the local-only setting is now set to the toots local-only attribute, which also explains #2431 (comment)
I'm still for continuing threaded mode on the last tooted toot after an edit, but left that change out in favour of this issue getting fixed.
Note: manually removing the local-only emoji is no longer possible.

There was both an issue with threaded mode and local-only toots when editing.
When activating either option, it was preserved on edit, but changing the local-only attribute is not supported and threaded mode got stuck editing the same toot.

The local-only option will be set to the toots local-only attribute to reflect that it is a local-only toot in the compose form. This unfortunately adds an additional local-only emoji.

Signed-off-by: Plastikmensch <[email protected]>
An additional local-only emoji was added to toots after editing a local-only toot, because the `do_not_federate` option is `true` and a local-only emoji is added unconditionally in such a case.

Signed-off-by: Plastikmensch <[email protected]>
@Plastikmensch Plastikmensch force-pushed the fix-threaded-mode-trapped-in-edit branch from eb727ff to f88e33d Compare October 31, 2023 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing post with threaded mode continues to edit the post
2 participants