-
Notifications
You must be signed in to change notification settings - Fork 259
-Created UI functionality for changing room name #59
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,23 +2,44 @@ import React from 'react' | |
import style from './index.module.css' | ||
|
||
export const RoomHeader = ({ | ||
state: { room, user, sidebarOpen, userListOpen }, | ||
actions: { setSidebar, setUserList }, | ||
state: { room, user, sidebarOpen, userListOpen, inputIsOpen, roomTitle }, | ||
actions: { setSidebar, setUserList, setTitleEditable, updateRoom, changeInputTitle }, | ||
}) => ( | ||
<header className={style.component}> | ||
<button onClick={e => setSidebar(!sidebarOpen)}> | ||
<svg> | ||
<use xlinkHref="index.svg#menu" /> | ||
</svg> | ||
</button> | ||
<h1>{room.name && room.name.replace(user.id, '')}</h1> | ||
{room.users && ( | ||
<div onClick={e => setUserList(!userListOpen)}> | ||
<span>{room.users.length}</span> | ||
<header className={style.component}> | ||
<button onClick={e => setSidebar(!sidebarOpen)}> | ||
<svg> | ||
<use xlinkHref="index.svg#members" /> | ||
<use xlinkHref="index.svg#menu" /> | ||
</svg> | ||
</div> | ||
)} | ||
</header> | ||
) | ||
</button> | ||
{ | ||
(room.name && !inputIsOpen) && <h1 onClick={e => setTitleEditable(!inputIsOpen, room.name)}> | ||
{room.name.replace(user.id, '')} | ||
</h1> | ||
} | ||
|
||
{ | ||
inputIsOpen && | ||
<h1>{[ | ||
<form > | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good practice when using forms is to add an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lukejacksonn okay I'll add it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also call the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly are you implying , I am missing your point here, do you want me to remove the svg and add a button & I haven't used onSubmit prop, I did use it in redux forms but that was different scenario. Can you elaborate a bit more ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is a form that handles both the user hitting enter and them clicking the submit button. This way the input it not controlled but left to do its thing until the user submits the form. Which requires less wiring (no
Does that make sense? What do you think? |
||
<span className={style.horizontal}> | ||
<input onChange={changeInputTitle} className={style.inputComponent} value={roomTitle} /> | ||
| ||
<svg onClick={e => updateRoom(room.id, roomTitle)}> | ||
<use xlinkHref="index.svg#done" /> | ||
</svg> | ||
</span> | ||
</form> | ||
]}</h1> | ||
} | ||
|
||
|
||
{room.users && ( | ||
<div onClick={e => setUserList(!userListOpen)}> | ||
<span>{room.users.length}</span> | ||
<svg> | ||
<use xlinkHref="index.svg#members" /> | ||
</svg> | ||
</div> | ||
)} | ||
</header> | ||
) |
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.
You could un-nest this a bit.. perhaps what we should be aiming for is something like:
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.
@lukejacksonn are you talking about changing the indentation or logic here ?
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.
Just not wrapping things so much.. like, having a
form
in anh1
feels a bit wrong.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 had wrapped the form in h1 because h1 had styles properties that were used by the heading before editable field is set true. So what I can do now is that to make another style object with same properties and apply it to form. But it will be like duplicating things. Apart from that I don't think that we need a form here either. If I remove it it'll work fine. So, should I remove it ?