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.

Sign up for GitHub

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

Jump to bottom

Add button to add channel #2013

Merged
merged 6 commits into from
Nov 6, 2020
Merged

Add button to add channel #2013

merged 6 commits into from
Nov 6, 2020

Conversation

Sheikah45
Copy link
Member

Fixes #1990

@Brutus5000
Copy link
Member

Brutus5000 commented Nov 4, 2020 •
edited
Loading

Tested and works, but not everybody is familiar with IRC, so I think we should emphasize that it needs to start with a # (or show an error otherwise instead of just blanking the text field)

@Sheikah45 Sheikah45 force-pushed the feature/#1990-add-channel-button branch from 80d09ff to 8f6d52a Compare November 4, 2020 22:43
@bukajsytlos
Copy link
Member

question is if the user is bother by the underlying chat technology? should we abstract that and simply prefix automatically?

@Sheikah45
Copy link
Member Author

Currently it gives the user an error if it doesnt start with #. I am not sure about adding the abstraction since the /join command would require the # still and it might make people think they don't need it there.

1-alex98
1-alex98 previously approved these changes Nov 5, 2020
View reviewed changes
Copy link
Member

@1-alex98 1-alex98 left a comment

Choose a reason for hiding this comment

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

I like maybe Kubko is correct and we should add the # automatically but otherwise quite good. log statement should probably not be warn level

@Brutus5000
Copy link
Member

No we shouldn't otherwise it's not consistent with the /join command also doesn't differentiate between chatrooms and private chat.

@Sheikah45 Sheikah45 merged commit 28e15e7 into develop Nov 6, 2020
Sheikah45 added a commit that referenced this pull request Nov 6, 2020
@Sheikah45
Add button to add channel (#2013)
cff0597
@Sheikah45 Sheikah45 linked an issue Nov 9, 2020 that may be closed by this pull request
Allow to join channels not only if all channels are closed #532
Closed
@Sheikah45 Sheikah45 deleted the feature/#1990-add-channel-button branch November 27, 2020 16:45
mrchris2000 pushed a commit to mrchris2000/downlords-faf-client that referenced this pull request Apr 15, 2022
@Sheikah45
Add button to add channel (FAForever#2013)
70fc922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1-alex98 1-alex98 1-alex98 left review comments

@Brutus5000 Brutus5000 Awaiting requested review from Brutus5000

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Join channel via a plus button as last tab Allow to join channels not only if all channels are closed
4 participants
@Sheikah45 @Brutus5000 @bukajsytlos @1-alex98