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

Nachrichten - Rework #218

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

Nachrichten - Rework #218

wants to merge 45 commits into from

Conversation

kurwjan
Copy link
Collaborator

@kurwjan kurwjan commented Apr 29, 2024

Die alte Nachrichten ( - Beta-Version) PR will ich nicht für immer zum vergammeln hier lassen, somit wird es jetzt gemacht.

Main TODOs im Code:

  • App Bar stylen
  • Nachrichten Builder neustrukturieren, ist gerade sehr unnötig bloated
  • Lanis-Style Formatting für Nachrichten
  • Bubble und Author Header richtig gestyled
  • Nachrichten mit Halten kopieren
  • DateHeader stylen
  • Rich Text Editor, da Nachrichten eher wie eine schlechtere Version von E-Mails benutzt wird.
    • ich muss ihn noch weiter testen ob er stabil genug ist
  • Konversations Erstell Prozess anpassen
    • Also Erstell-Dialog, direkt zum Rich Text Editor schicken, ...
  • MessageState Style
  • Optimieren, aufräumen, ...
  • Translations

Andere issues:

  • Bubblegröße an die Gerätebreite anpassen.
  • Wenn man eine Nachricht sendet wird immer nur mit MessageState.first gesendet
  • Snackbar bei unerfolgreichen senden.
  • Keine Auswahl an Nachrichtentypen wenn das nicht erlaubt wurde
  • Eigene Datenklassen statt dynamic beim Fetcher
  • Dispose()?
  • Teilnehmer sehen

Potenzielle Performance issues:

  • MessageBuilder zu einem Stateful Widget umwandeln, damit z. B. SetState-Calls nicht alles rebuilden
  • Jedesmal BubbleStyle, BubbleStructure und FormatStyle auszurechnen ist nicht effizent, aber sollte fast gar keine Performance fressen. Trotzdem sollte man es vorher z. B. beim globalen Theme-Init speichern.
    • Hab es jetzt nur bei BubbleStyle gemacht, weil da bringt es glaube auch was.
  • LinkedList anstatt eines Arrays?
    • Wenn man eine neue Nachricht sendet muss man immer den voherigen Item überprüfen, also ich weiß ob es dann so viel bringt. Bringt nix mehr
  • Zur Impeller-Engine wechseln, mache wahrscheinlich ein eigenes Issue dazu.

+ May not follow project "guidelines"
@kurwjan kurwjan added the todo something to do label Apr 29, 2024
@kurwjan kurwjan self-assigned this Apr 29, 2024
@kurwjan kurwjan changed the title Nachrichten - Senden und Erstellen Nachrichten - Antworten und Erstellen Apr 30, 2024
@alessioC42
Copy link
Owner

UI/UX wird bisschen komplexer weil man müsste eine halbwegs gute Messaging-UI bauen.
Villt. gibt es schon Packages für sowas

flutter_chat_ui sieht sehr solide aus. Das Example ist wirklich einfach und soweit ich das sehe, kann man über textMessageBuilder auch eigene sachen sehr gut einbauen (hier vor allem markup)

@kurwjan kurwjan linked an issue May 3, 2024 that may be closed by this pull request
@kurwjan kurwjan added enhancement New feature or request and removed todo something to do labels May 3, 2024
@alessioC42
Copy link
Owner

Ich denke es wäre hier super hilfreich, wenn anstelle von dynamic eigene klassen für alles erstellt werden. Das hat mir bei den substituzions sehr geholfen und ist sehr viel cleaner.

@kurwjan
Copy link
Collaborator Author

kurwjan commented May 4, 2024

Ja das ist eine gute Idee

@kurwjan
Copy link
Collaborator Author

kurwjan commented Jun 23, 2024

Das ist schon sehr groß geworden. 1.000 Zeilen an neuen Code.

@kurwjan kurwjan marked this pull request as ready for review July 18, 2024 18:41
@kurwjan
Copy link
Collaborator Author

kurwjan commented Jul 18, 2024

Ich glaube das ist jetzt endlich ready. Ich hoffe ich habe nichts vergessen.
overview.dart ist jetzt halt bissl messy (war schon davor), aber das wird mal wann anders aufgeräumt.

@kurwjan kurwjan changed the title Nachrichten - Antworten und Erstellen Nachrichten - Rework Jul 18, 2024
@alessioC42
Copy link
Owner

alessioC42 commented Jul 18, 2024

Ich brauche vlt ne weile mit dem durchschauen ^^
image
Habe gerade wegen bewerbungen bissel was am hut. Drängt ja aber auch nicht wegen ferien

+ Removed during a debug removal commit
+ Now also set position to the bottom
@alessioC42 alessioC42 self-requested a review July 28, 2024 16:46
Copy link
Owner

@alessioC42 alessioC42 left a comment

Choose a reason for hiding this comment

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

Could you also specify the return type of client.substitutions.getOverview with a special data class. I where not able to mark that with the review tool in InteliJ since it was not changed

Looks good overall, I will implement the UI things I talked about, but seems solid (or as solid as it can be for our situation).

app/lib/client/client_submodules/conversations.dart Outdated Show resolved Hide resolved
app/lib/view/conversations/send.dart Outdated Show resolved Hide resolved
app/lib/view/conversations/send.dart Outdated Show resolved Hide resolved
@kurwjan
Copy link
Collaborator Author

kurwjan commented Jul 30, 2024

I wanted to do the overview thing later in a separate PR where I also rework it but I'll look into it.

The scroll to bottom FAB doesn't show itself directly, only if I scrolled down; I'll look into that.
Maybe starting at the bottom is better and then you have a button which brings you to the top or the chat is reversed (nah that would be weird)?

Und soll ich jetzt Englisch oder Deutsch hier auf Github benutzen, weil irgendwie wird das immer herumgeswitcht.

+ Revert remove text button to delete_all
+ Removed back button in send.dart
+ searchTeacher() function now returns only a receiverTeacher class and added the fromJson() constructor to it.
+ ConversationParser class stores result of the canChooseType() function
+ Put type chooser dialog into a Scaffold
@kurwjan
Copy link
Collaborator Author

kurwjan commented Jul 30, 2024

So now the UI needs to be adjusted bc the dialogues were just put in a Scaffold without really changing them and exceptions.

+ Subject text field expands
+ Add receiver field received icons, etc.
+ Removed buttons, added FABs
+ Remove borders, add elevation => fits better
+ Receivers have now icons whether they're a group, teacher or etc.
+ Replace beautiful ExpansionTiles with boring RadioListTiles
+ Put dialog functions into Stateful Widgets
+ Fix mistake in overview ErrorView (!content -> !error)
+ Adjust translation for no connection
+ Replace fetcher parameter with function parameter in ErrorView
+ Add AppBar to ErrorView
+ Add report buttons to ErrorView
+ If NoConnectionException occurred, display "No internet connection" instead of a critical error screen
@kurwjan
Copy link
Collaborator Author

kurwjan commented Aug 2, 2024

I think it's ready. After that the overview needs to be cleaned and maybe then I can do experimental ideas like a dashboard?

@kurwjan kurwjan requested a review from alessioC42 August 2, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nachrichten interagierbar machen
2 participants