-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Room UI Redesign #3006
Room UI Redesign #3006
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new design looks great! Left some comments (some on design, others about implementation). Some other thoughts that came to mind:
-
Strings should come from translation files. Might not be necessary in the storybook stories (though still seems desirable). The more important place they should be used is in the components themselves.
-
Now may be a good time to ask accessibility-minded folks for feedback. There has been a very helpful group submitting issues for some time, and I suspect they will be happy that we are designing with better accessibility in mind from square one. For that reason, it might be worth serving storybook on hubs.mozilla.com (via reticulum) (/ui or /storybook or something) so that they don't have to be developers to participate in the design/review loop.
-
I get a variety of React errors when clicking thru storybook -- I'm not sure if they're all expected of there might be bugs. Probably worth going thru them and making sure none of them are surprising.
<div className={classNames(styles.toolbar, className)} {...rest}> | ||
<div className={classNames(styles.content, styles.leftContent, styleUtils.showMd)}>{left}</div> | ||
<div className={classNames(styles.content, styles.centerContent)}>{center}</div> | ||
<div className={classNames(styles.content, styles.rightContent, styleUtils.showMd)}>{right}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find where leftContent
, centerContent
, and rightContent
are defined in scss. Are these unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove these, thanks 👍
style={{ | ||
clip: `rect(${micButtonDiameter - | ||
Math.floor(micLevel * micButtonDiameter)}px, ${micButtonDiameter}px, ${micButtonDiameter}px, 0px)` | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icon={<ObjectIcon />} | ||
selected={popoverVisible} | ||
onClick={togglePopover} | ||
label="Place" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what the best word for it would be ("create", "spawn", "share", "post" and "add" all have their own issues), but "place" is very surprising to me -- might be an issue worth revisiting
</Button> | ||
<Button preset="orange" onClick={onSpectate}> | ||
<ShowIcon /> Spectate | ||
</Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to emphasize "Join Room" more than "Enter On Device" and "Spectate". Seems like making this first choice easy will help newcomers (who may not have any conception of what a room is, what enter on device means, or what spectate does), and even beyond that, I suspect "Join Room" is the most common action by a significant margin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also curious to see how this would look if the icons were vertically aligned with on another
<ToolbarButton | ||
icon={<Icon />} | ||
onClick={() => onSelect(item)} | ||
label="Share" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Share" seems ambiguous (especially on mobile). Have you considered alternatives? How about "stream"?
@@ -0,0 +1,50 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this IconTemplate work?
Room UI Redesign: Add setting to hide/show FPS counter
Room UI Redesign: Media Browser
Room UI Redesign: Fix attach button in chat sidebar
Room UI Redesign: Add leave room item to more menu
Room UI Redesign: Add send button
Room UI Redesign: Fix popover padding on mobile
Room UI Redesign: Fix ChatSidebar font size and link styling
Room UI Redesign: React / Place Menu Permission Checks
Room UI Redesign: Landing Page and Static Page Styling
Close the sidebar when you go to object on small screens
New UI RTC retry fixes
Room UI Redesign: Miscellaneous Fixes
Room UI Redesign: Fix attribution rendering
Room UI Redesign: Miscellaneous Fixes Part II
Room UI Redesign: Fix landing page app description styling
We now have a new look and feel for Hubs! This UI update includes a new desktop and mobile interface for the Hubs client to make it easier than ever to use Hubs on different devices. You can read more about the update here.
This is the integration branch for the Room UI Redesign.
Merged PRs:
Storybook JS and Button
Toolbar
Popover
Text Inputs and Input Accessibility
Invite Popover
Share and Place Popovers
More Menu
Code Cleanup
Room Entry Modal
Enter on Device Modal
Add User Modals and Sidebars
Microphone Setup Modals
Loading Screen
Entry Flow
More Menu
Chat Sidebar
People Sidebar
Object List and Object Menu
Toolbar Buttons / Menus
Room Settings Sidebar
Room Info Sidebar
Sign In Modal / Auth UI Refactor
Promote Client Modal
Add inspect other player behavior
Auto Exit Modal and Exited Screen
Remove Help and Support Dialogs
Safari Mic Modal
Leave Room Modal
Reaction Menu
Refactor Room Sidebar
Safari Fixes
Remove old components and migrate to using Column
Reduce Custom CSS
Migrate Modals
To Do:
*
= Non Blocking