Open xtendo-org opened 8 years ago
bump, would love this
Hey, thanks a lot for your contribution!
This looks good to me, simple feature (almost a bugfix), simple PR. I give my :+1:, can @JocelynDelalande, @erming or @floogulinc take the second look here please?
Thanks for the contrib @kinoru
Technically, this commit is ok, and does what it says :)
However, I have to say I'm not a big fan of the approach :
What I would prefer is to push a modification on theme/UI to make this accidental quitting behaviour disappear.
Then, and after some real-use of this improved UX, we would be on the proper ground to decide if we want a confirmation on channel leaving.
@kinoru @astorije what do you think ? @kinoru Would you be ok to submit some UI/theme improvement to get rid of that accidental closing ?
Similar to that, an "undo" action might be a good way to solve this. If you would delay the actual leaving of the room by a couple of seconds in the backend, and have a toast of some sort pop up with an "undo" button that will cancel the leaving and re-add the room back to the list.
@YaManicKill yeah, quite "perfect" solution IMHO, but as a lot of perfect solutions, it requires some dev, and cautious UX testing, so… time :-) where we have here an annoying issue to solve (IMHO) quite quickly.
@YaManicKill got some motivation/skils/time to work on an undo UI ? [Mailpile is using such a notification+undo, it's quite pleasant to use].
I might take a look at it. There are a few other things that I want to do first that annoy me much more (lack of offline mode mainly). But I'll definitely take a look at it and create a PR from my own fork.
I already have a few things that I've done on my fork that I want to get pushed up here as PRs as well.
I already have a few things that I've done on my fork that I want to get pushed up here as PRs as well.
Go ahead.
Good things coming :)
Accidently clicking the close button has never been a problem for me but maybe just have an option to turn off the close buttons on the channel list, leaving only the leave button in the top right when you are on a channel.
I agree with @JocelynDelalande, @YaManicKill, and @floogulinc.
- whether there is a confirmation or not, a theme designed in a way that frequently leads to clicking on close button is a frustrating thing.
Yes, I personally believe that we shouldn't have the hover-appearing "X" button at all, and it should be disabled by default for the sake of beginners (Perhaps with an option to enable it). Or maybe we can replace it with the "middle click to close" behaviour which is exactly what happens with web browsers.
The idea of delayed undoable action is brilliant. I think it would be lovely to have that feature for the "Leave" button within the channel chat screen.
@kinoru @astorije what do you think ?
I think that yes, what you guys have suggested is ideal. But I also think that there is the non-ideal solution in a PR, and the ideal solution nowhere yet. Let's not make the mistake to refuse the good enough hoping for a better, please!
If someone comes up with a better solution, perfect, they can also remove that workaround at the same time! Until this happens, @kinoru's solution Just Works™.
For example, having an "Undo" button, while a terrific solution, is also immensely difficult to get right. Simply delaying the command by 30 seconds, while simple, is not good: what happens if I leave my browser within these 30 seconds (like, power outage, no JS events can be sent)? Then this move to the server, and... before you know it, it's very complex, requires heavy testing, etc. Again, while not ideal, we have something here, and nothing is set in stone.
That's what I think :-)
maybe just have an option to turn off the close buttons on the channel list, leaving only the leave button in the top right when you are on a channel.
Nah, this is a UI choice. We offer (and will offer better later) ways to produce alternative themes, I don't want a settings page overly complicated where you can decide on everything. This is not emacs :D (trolling for free) Even though I have never run into this situation I believe, I'm OK with fixing this as part of the theme rather than offering an option as a workaround.
@astorije's last comment inspired me to make this commit which is even simpler and hopefully a reasonable ground upon which we can all agree for the time being. It is, I believe, also exactly what @JocelynDelalande asked.
So this commit changes the stylesheet to not show the close button by default. Users who want it can easily bring it back by adding to their theme:
#sidebar .close { display: block; }
I do not think this is a problem really. The close button isn't easy to accidentally hit and is pretty obvious when you are mousing over the channels. Unless a user literally isn't looking at the sidebar when they are clicking on it, this shouldn't be a problem. If a user doesn't like the close buttons, they can disable it in their own theme.
I disagree, I think that it can be confusing, I did it a few times myself at the beginning of using Shout. However, the simplest solution to the confusion would be the default theme showing them all the time, or hiding them all the time.
Might also help to move the close buttons to the far side of the channel list, so they aren't between the chat and the channels.
Oh yes, I like that idea. Especially if they are always visible.
Might also help to move the close buttons to the far side of the channel list, so they aren't between the chat and the channels.
This quoted solution seems reasonable and really simple, @kinoru what do you think about it ?
I think that yes, what you guys have suggested is ideal. But I also think that there is the non-ideal solution in a PR, and the ideal solution nowhere yet. Let's not make the mistake to refuse the good enough hoping for a better, please!
Yup, I agree with the philosophy, but here, it clutters the UI for everyone to fix something which is an issue for some people (and thus must be adressed, don't get me wrong).
Regarding the undo solution, I do agree @astorije (as expressed lightly before) that if no one is going to spend time right now on undo mechanism, another solution is needed in the meantime :-)
I would suggest completely removing the close button in serverlist, it doesn't work on mobile, and there's already a dedicated 'Leave' button next to the user list button.
Plus there's #613 open to add a context menu (although it doesn't work on mobile as of now, but that should be possible to fix).
I would suggest completely removing the close button in serverlist, it doesn't work on mobile, and there's already a dedicated 'Leave' button next to the user list button.
I don't know, that's a global UI change that should be discussed IMHO.
@kinoru any input on suggested solutions ?
Regretfully, we're not approaching at least a temporary conclusion, but more people are introducing more options on the table and the discussion is inching away from consensus instead of nearing it. I'm worried if this is a typical "What color should the bike shed be" situation. I personally think all solutions suggested so far are good and will do the job of preventing accidental channel part. Mine is simple, and it's not superior than any other option in any other way.
Just as @astorije has already pointed out, the truly ideal one is not to appear any time soon, and my intention is that the currently pull-requested commit serves to address the issue until it happens.
It's not like the decision would be irrevocable. We can always invalidate this one as soon as we get a better option or reach a new conclusion. So, why can't we just merge this pull request for the time being, and further discuss other options like global UI change (undo-able parting, close button on the far side, complete removal, etc.) in a separate issue? I'm pretty sure this temporary commit will not harm the user experience in the meantime.
Regretfully, we're not approaching at least a temporary conclusion, but more people are introducing more options on the table and the discussion is inching away from consensus instead of nearing it. I'm worried if this is a typical "What color should the bike shed be" situation. I personally think all solutions suggested so far are good and will do the job of preventing accidental channel part. Mine is simple, and it's not superior than any other option in any other way.
IMHO UX decisions need to be discussed (or better, tested, but that's another story). So, regarding your accidental click issue it's bike-sheding, but from global ergonomy and user feeling standpoint, that's not bike-shedding.
Just as @astorije has already pointed out, the truly ideal one is not to appear any time soon, and my intention is that the currently pull-requested commit serves to address the issue until it happens.
Absolutely agree on that. We're looking for few-lines-patch solution.
It's not like the decision would be irrevocable. We can always invalidate this one as soon as we get a better option or reach a new conclusion. So, why can't we just merge this pull request for the time being, and further discuss other options like global UI change (undo-able parting, close button on the far side, complete removal, etc.) in a separate issue? I'm pretty sure this temporary commit will not harm the user experience in the meantime.
shout has good UX, so changing/cluttering the UI is a light decision in terms of code but not in terms of user feeling…
We have here 4 "light" solutions :
I'd just go for 3. because it solve the issue, has the smallest negative impact on UX and won't loose any user. I think we have an objectively better (setting aside tastes and shed color) solution, is that a shared point of view ?
@JocelynDelalande Just to make sure: I've dropped the "ask confirmation" solution. The current commit (display: none;
) is more like complete removal suggested by @xPaw.
Moving the close button to the left sounds unnatural because I've never seen any browser having the tab close button on the left side, but I'm not against it at all as long as someone does the CSS tweaking required for it. My opinion is that we merge this pull request until that happens.
Hiding it is, as @JocelynDelalande pointed out, a considerable change to the day-to-day usage of shout. I propose that we NOT merge this PR, or any that hides a commonly used control, until there is consensus. On this issue, there is not consensus.
@kinoru As for browser close-tab buttons… Not that it's necessarily relevant (the display layout is completely different, a horizontal string of elements versus a vertical stack), but there are browsers that put the close button on the left. Safari (at least on OS X) does so, and in fact OS X as a whole places window controls in the top left corner. I've used Linux systems with top-left controls as well; it usually depends on one's windowing system theme. At any rate, even if putting a close button on the left is unnatural, I think it's more unnatural to place the control in a location where it can be so easily activated by accident.
Don't forget there's a dedicated button "Leave" next to the user list button.
@xPaw That requires switching to the channel in question, does it not? Shouldn't be necessary IMO.
@dgw It does. But with addition of context menu, that would be fixed.
I'm just trying to think of other chat clients, and I can't think of any that have a close button in user/channel list.
Off the top of my head, KiwiIRC does.
Closing a channel isn't really something people do when they are in another channel, is it?
raises hand timidly
But I am a strange one.
My friends to whom I've recommended Shout are all accidentally leaving channels. This is because in default theme, the channel close button will only show up when the mouse pointer is on the channel name, and beginners don't know that they exist at all. Even after they learn the existence of such button, it takes more unwanted channel leavings to finally be trained to avoid it.
This is avoidable by changing the theme, but it would be nicer if the default just worked better. Currently only the server disconnect action is asked with confirmation. This commit adds such confirmation for the channel part action as well.
This commit does not add such prompt to query close buttons.