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

Feature/edit messages #3608

Merged
merged 28 commits into from
Feb 19, 2024
Merged

Feature/edit messages #3608

merged 28 commits into from
Feb 19, 2024

Conversation

sowjanyakch
Copy link
Contributor

@sowjanyakch sowjanyakch commented Jan 30, 2024

Resolves #3509

  • Editing a message in a group of messages split the group at the edited message.
  • After completing edit or aborting it, it should resume the previous input.
  • Edit in default input showing the original content as a "quote"
    • Don't allow changing from message to caption and vice-versa
    • Don't allow to toggle silent send on
  • System message hidden like with delete?
  • Visible indicator a message was edited (and by whom/when)?
    • Amend to the author name (edited) / (edited by moderator-name)
    • Editor+Time in Message menu as subline of timestamp
  • Time limitation like for delete?
    • Limit editing to X hours (draft 24 hours, check what others do and discuss again)
    • Set the limit on deletion to infinite
  • What to do about mention notifications (added a mention, removed a mention)?
    • Should indicate in the UI what happens (only when affected)
    • Make sure people don't get 1 for the message and 2 for followup edits
  • What about bots?
    • Can not edit bot messages, moderators can
    • Bots will not get explicitly notified about previous edits (only with the default system message)?
    • If a message triggered a bot already it's yolo and will not be retriggered with the new content automatically (bot can do based on the system message?)
      • Should indicate in the UI what happens (only when affected)

🖼️ Screenshots

edit

chats

edit message dialogue

🚧 TODO

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

sowjanyakch and others added 4 commits January 19, 2024 18:12
Signed-off-by:Sowjanya Kota <101803542+sowjanyakch@users.noreply.github.com>
Signed-off-by: Sowjanya Kota <101803542+sowjanyakch@users.noreply.github.com>
Signed-off-by: Sowjanya Kota <101803542+sowjanyakch@users.noreply.github.com>
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
Signed-off-by: Sowjanya Kota <101803542+sowjanyakch@users.noreply.github.com>
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
@sowjanyakch sowjanyakch marked this pull request as draft January 30, 2024 10:11
Signed-off-by: Sowjanya Kota <101803542+sowjanyakch@users.noreply.github.com>
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
Signed-off-by: Sowjanya Kota <sowjanya.kch@gmail.com>
Signed-off-by:Sowjanya Kota <sowjanya.kch@gmail.com>
@mahibi
Copy link
Collaborator

mahibi commented Feb 2, 2024

message editing itself works good already 👍 👍

apart from the other things you're already working on, just some notes:

  • add error message to onError (log, and snackbar for UI)
  • hide edit message option from menu for message types other than textmessages (location, voice message , ...)
  • hide file attachment icon when being in editing mode
  • modify color of the checkmark icon to respect dark/light mode. for example change ic_check_24.xml to:
<!--
    @author Google LLC
    Copyright (C) 2021 Google LLC

    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
    You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

    Unless required by applicable law or agreed to in writing, software
    distributed under the License is distributed on an "AS IS" BASIS,
    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    See the License for the specific language governing permissions and
    limitations under the License.
-->
<vector xmlns:android="http://schemas.android.com/apk/res/android"
    android:width="24dp"
    android:height="24dp"
    android:viewportWidth="24"
    android:viewportHeight="24"
    android:tint="?attr/colorControlNormal">
    <path android:fillColor="@color/fontAppbar" android:pathData="M9,16.17L4.83,12l-1.42,1.41L9,19 21,7l-1.41,-1.41z"/>
</vector>
  • change color for "Edit Message text", pen icon, X icon. i'm not sure about the colors yet. maybe grey?
  • the checkmark icon to send the editied message should be themed, like the send button in normal mode.
    just have a look in themeMessageInputView how it's done for other components.
    you get a feeling for theming when you change color theme in web:
    grafik
    then you should recognize that the mobile app also changes colors.
    Components in the android app can be adapted for themeing by using the ViewThemeUtils.
    Just compare how it's done for other components in themeMessageInputView()
  • lint warnings are increased (see github report, you can click on "master" and "PR" to compare)
  • Formatting in NcApi.java went wild somehow. best is to revert his and only change the lines for this PR.

just let me know in chat if we should have a look together for any topic..

sowjanyakch and others added 17 commits February 2, 2024 17:09
Signed-off-by: Sowjanya Kota <sowjanya.kch@gmail.com>
…ure/edit_messages

# Conflicts:
#	app/src/main/java/com/nextcloud/talk/api/NcApi.java
#	app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
#	app/src/main/java/com/nextcloud/talk/ui/MessageInput.kt
#	app/src/main/java/com/nextcloud/talk/ui/dialog/MessageActionsDialog.kt
#	app/src/main/res/drawable/ic_check_24.xml
#	app/src/main/res/values/strings.xml
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
This will update the message when an edit was made on other devices.
So the system message will trigger that you are informed about a change. But instead to show the system message, you use it's information to immediately update the adapter.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: sowjanyakch <sowjanya.kch@gmail.com>
@mahibi

This comment was marked as outdated.

@mahibi
Copy link
Collaborator

mahibi commented Feb 14, 2024

Bildschirmfoto vom 2024-02-14 10-19-59 geaendert

i suggest to move "edited" before the checkmark? like:
Test2 10:19 (edited) ✔️

and also apply the same color to the "(edited)" like for the time

@mahibi
Copy link
Collaborator

mahibi commented Feb 14, 2024

  • reminder to myself: branch off stable-18.1.0 before merge this PR

Signed-off-by: Sowjanya Kota<sowjanya.kch@gmail.com>
Signed-off-by: Sowjanya Kota<sowjanya.kch@gmail.com>
Copy link
Contributor

Codacy

Lint

TypemasterPR
Warnings8084
Errors88

SpotBugs

CategoryBaseNew
Bad practice66
Correctness88
Dodgy code112112
Internationalization33
Malicious code vulnerability33
Performance66
Security11
Total139139

Lint increased!

Copy link
Contributor

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/3608-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

@mahibi mahibi assigned mahibi and sowjanyakch and unassigned mahibi Feb 19, 2024
@mahibi mahibi added this to the 19.0.0 milestone Feb 19, 2024
@mahibi mahibi merged commit e5f25bd into master Feb 19, 2024
13 of 16 checks passed
@mahibi mahibi deleted the feature/edit_messages branch February 19, 2024 09:12
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.

✏️ Edit message - Android
2 participants