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

0.11 feat realtime review sev #548

Draft
wants to merge 4 commits into
base: 0.11-feat-realtime
Choose a base branch
from

Conversation

Severino
Copy link
Contributor

  • Reworked comment section in store from handleComment => addComment,updateComment and deleteComment respectively. Functions should only have one task and this is more readable.
  • Fixed a problem with the created comments being send to the creator himself, resulting in a duplicated entries
  • Reworked the activeUserIndicator in style and excluded the current user, as the current user should be aware he is on that site

@Severino
Copy link
Contributor Author

Severino commented Dec 13, 2024

activeUserIndicator

Removed the active user from the field. added a border and made the dot blinking to increase the visibility.

@v1r0x
Copy link
Member

v1r0x commented Dec 16, 2024

My opinion regarding the active user list:

  • I'd keep the current user, but we can hide the whole list if they are the only user
  • I'd also keep the green dot indicator, because it is the default indicator for online in (almost) every other website/app/program
  • The border breaks with the rest of the row layout and draws focus. I tried a more subtle design with an almost transparent background and no border, but IMO no border and no background looks best and matches the other parts
  • The blinking dot is also distracting and draws focus

active-users

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.

2 participants