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

User List UI Component #84

Merged
merged 7 commits into from
Oct 19, 2021
Merged

User List UI Component #84

merged 7 commits into from
Oct 19, 2021

Conversation

snicki13
Copy link
Member

@snicki13 snicki13 commented Oct 19, 2021

This is an UI only PR.


Part of #55

@snicki13 snicki13 requested a review from mxsph October 19, 2021 11:11
@snicki13 snicki13 enabled auto-merge (squash) October 19, 2021 11:11
@snicki13 snicki13 added the UI/UX label Oct 19, 2021
Copy link
Member

@mxsph mxsph left a comment

Choose a reason for hiding this comment

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

Looks good so far 👍 However, I still have three comments:

  • If you set yourself to invisible it leads to an overflow of the container which in turn leads to a scrollbar being displayed.
    error

  • I would change the icons (possibly the icons from the mockup). Also, I would add tooltips here to describe what the icons do.
    grafik

  • I would also make these changes to fix overflow errors when a username is very long:

diff --git a/web-gui/src/app/page-components/classroom/ticket-list/ticket/ticket-user-display/ticket-user-display.component.scss b/web-gui/src/app/page-components/classroom/ticket-list/ticket/ticket-user-display/ticket-user-display.component.scss
index 9c71898..afb1c22 100644
--- a/web-gui/src/app/page-components/classroom/ticket-list/ticket/ticket-user-display/ticket-user-display.component.scss
+++ b/web-gui/src/app/page-components/classroom/ticket-list/ticket/ticket-user-display/ticket-user-display.component.scss
@@ -4,7 +4,7 @@
   align-items: center;
   min-width: 132px;
   max-width: 200px;
-  height: 38px;
+  min-height: 38px;
   .user-avatar {
     height: 32px;
     margin-right: 5px;
@@ -20,6 +20,7 @@
     }
     .user-name {
       font-size: 16.4px;
+      word-wrap: break-word;
     }
   }
 }

@snicki13
Copy link
Member Author

I added the tooltips and hid the overflow.
However:

  • Your suggested change has no effect
  • I will keep the icons as they are right now, because the focus of these icons is to call or join the user. The icons form the mockup shift the focus to the conference in my opinion.

btw.: I know, that the component structure is somewhat crazy right now. Refactoring will definitely happen.

@snicki13 snicki13 requested a review from mxsph October 19, 2021 13:00
@snicki13
Copy link
Member Author

We can discuss the icons at the next meeting :)

@mxsph mxsph changed the title User List User List UI Component Oct 19, 2021
@mxsph mxsph disabled auto-merge October 19, 2021 13:32
@mxsph mxsph merged commit cd358b4 into master Oct 19, 2021
@mxsph mxsph deleted the ui/user-list branch October 19, 2021 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants