various fixes to RoomHeader #880
Conversation
so unless I'm going insane, it should be a string.
fixes
```
rageshake.js:61 Warning: Failed prop type: The prop `onClick` is marked as required in `AccessibleButton`, but its value is `undefined`.
in AccessibleButton (created by RoomHeader)
in RoomHeader (created by RoomView)
in div (created by RoomView)
in RoomView (created by LoggedInView)
in main (created by LoggedInView)
in div (created by LoggedInView)
in div (created by LoggedInView)
in LoggedInView (created by MatrixChat)
in MatrixChat
```
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
pass inRoom prop to RoomHeader (defaults to false) remove default onSettingsClick, handle if it is passed EVERYWHERE if onSettingsClick is passes, show that button show search button only if we are in the room, seems to fail otherwise this seems to handle all cases I could throw at it. Give it your best Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
|
Can one of the admins verify this patch? |
| const innerName = | ||
| <EmojiText element="div" | ||
| className={ "mx_RoomHeader_nametext " + (settingsHint ? "mx_RoomHeader_settingsHint" : "") } | ||
| title={ roomName }>{roomName}</EmojiText>; |
There was a problem hiding this comment.
A few things here:
- props should be
{"someValue"}(no spaces) - children should be
{ someChild }
There was a problem hiding this comment.
Fair enough, I actually just copied that and did not change it. Will add a commit to remedy that
| @@ -225,12 +226,17 @@ module.exports = React.createClass({ | |||
| roomName = this.props.room.name; | |||
| } | |||
|
|
|||
| const innerName = | |||
| <EmojiText element="div" | |||
| className={ "mx_RoomHeader_nametext " + (settingsHint ? "mx_RoomHeader_settingsHint" : "") } | |||
There was a problem hiding this comment.
It'd be really awesome if this used classnames which can be imported from /src. Then the indentation could be a bit nicer 😀 (i.e. someClasses = classnames({...}) ... className={someClasses} )
| className={ "mx_RoomHeader_nametext " + (settingsHint ? "mx_RoomHeader_settingsHint" : "") } | ||
| title={ roomName }>{roomName}</EmojiText>; | ||
|
|
||
| if (this.props.onSettingsClick) { |
There was a problem hiding this comment.
I'm pretty sure it's ok to pass undefined to onClick, so this is a bit redundant.
There was a problem hiding this comment.
rageshake.js:61 Warning: Failed prop type: The prop `onClick` is marked as required in `AccessibleButton`, but its value is `undefined`.
in AccessibleButton (created by RoomHeader)
in RoomHeader (created by RoomView)
in div (created by RoomView)
in RoomView (created by LoggedInView)
in main (created by LoggedInView)
in div (created by LoggedInView)
in div (created by LoggedInView)
in LoggedInView (created by MatrixChat)
in MatrixChat
There was a problem hiding this comment.
But this isn't an AccessibleButton, it's a div so that error must be for the other thing.
| @@ -299,6 +305,14 @@ module.exports = React.createClass({ | |||
| </AccessibleButton>; | |||
| } | |||
|
|
|||
| let search_button; | |||
| if (this.props.onSearchClick && this.props.inRoom) { | |||
There was a problem hiding this comment.
Again I don't think we should be checking for the optional onSearchClick, let's allow AccessibleButton to decide whether to call undefined or not.
What even am I saying, a button without an onClick is quite useless. Ignore me whilst I miss the point entirely 😬
There was a problem hiding this comment.
This is so we don't show a search button if it's a no-op
It's confusing
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
#880 (comment) Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
|
given the changes are approved, why isn't this merged? :) |
and some smaller fixes
closes element-hq/element-web#3859