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

Implement a user profile in the desktop client #36

Merged
merged 111 commits into from
Jul 31, 2023

Conversation

Vladyslav-Kuksiuk
Copy link
Contributor

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk commented Jun 8, 2023

This PR implements a user profile info tab in the desktop client and modal dialogs.

Here are some usage scenarios:

  • Chat deletion and creation via profile:
    chat deletion and creation_Trim
  • Chat deletion via the 'More' button:
    ChatSPN 2023-06-22 13-44-13_Trim
  • Log out:
    ChatSPN 2023-06-22 13-47-58_Trim

Copy link

@Artem-Semenov-dev Artem-Semenov-dev left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

General UI idea is to have less pop-ups, so that users would not have any blocking windows unless something important happens (payment, deletion, etc.).

Please have a look at other chats, and analyse how they implement similar functionality.

I am not reviewing the code changes now, because the approach in general should be different.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk marked this pull request as draft June 20, 2023 10:00
# Conflicts:
#	client/src/main/kotlin/io/spine/examples/chatspn/desktop/Avatar.kt
#	client/src/main/kotlin/io/spine/examples/chatspn/desktop/ChatsPage.kt
#	client/src/main/kotlin/io/spine/examples/chatspn/desktop/Client.kt
Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@Vladyslav-Kuksiuk LGTM with some comments to address.

/**
* Observes the outcome of the command.
*
* When a success or failure event is emitted, subscriptions will be canceled.
Copy link
Contributor

Choose a reason for hiding this comment

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

"are cancelled"

Please DO have a read on English conditionals.

/**
* Opens a page with a user profile.
*
* @param userId id of the user to open profile
Copy link
Contributor

Choose a reason for hiding this comment

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

ID

Please do a case-sensitive search for "id" in the documentation.

}

/**
* Opens a page with chat info. If the chat is personal, a user profile will be opened.
Copy link
Contributor

Choose a reason for hiding this comment

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

The first paragraph should always be a single sentence.

* Finds chat in the list by ID.
*
* @param id ID of the chat to find
* @return found chat or `null` if chat is not found
Copy link
Contributor

Choose a reason for hiding this comment

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

"chat with the given ID, or null if ..."

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk merged commit b9d387d into master Jul 31, 2023
1 check passed
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk deleted the client/user-profile branch August 2, 2023 08:33
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.

3 participants