Skip to content

Limit scroll bar in sidebar to chat view#550

Merged
Ivansss merged 6 commits into
masterfrom
limit-scroll-bar-in-sidebar-to-chatview
Jan 10, 2018
Merged

Limit scroll bar in sidebar to chat view#550
Ivansss merged 6 commits into
masterfrom
limit-scroll-bar-in-sidebar-to-chatview

Conversation

@danxuliu

@danxuliu danxuliu commented Jan 9, 2018

Copy link
Copy Markdown
Member

Except when almost empty, the message list of the chat view is taller than its available vertical space in the sidebar. Due to this a scroll bar was shown for the whole sidebar, and everything was moved when
scrolling to see overflown messages.

That is a valid approach when messages are shown from newest to oldest and the new message input is above the list of messages, as in that case the call details and the tab headers are near the new message input and the newest messages.

However, once the layout of the chat view is reversed and the messages are shown from oldest to newest with the new message input below the list of messages having to scroll back and forth to see the call details/tab headers or the new message input/newest messages would be a pain.

Therefore now the scroll bar is shown only for the message list, so it can be scrolled without moving the other elements in the sidebar.

Limiting the scroll bar in the sidebar to the list of chat messages causes the scroll bar to be removed from the whole sidebar in other tabs too, so the scroll bars had to be explicitly enabled in the list of participants.

Note that now, due to the scroll bar being shown only for the tabs, the call details are always visible. In short screens this may not leave too much space for the tabs, making them uncomfortable to use. This will be fixed in another pull request.

@danxuliu danxuliu added 3. to review feature: chat 💬 Chat and system messages labels Jan 9, 2018
@danxuliu danxuliu added this to the 3.0 (Nextcloud 13.0.0) milestone Jan 9, 2018
Except when almost empty, the message list of the chat view is taller
than its available vertical space in the sidebar. Due to this a scroll
bar was shown for the whole sidebar, and everything was moved when
scrolling to see overflown messages. Now the scroll bar is shown only
for the message list, so it can be scrolled without moving the other
elements in the sidebar.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The class was copied from the markup of the server sidebar, but it is
not used anywhere and its name is misleading now that the scroll bar is
shown on the tabs that need it.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
In CSS it is not currently possible to select an element based on
whether it has certain child or not, nor select the parent element of
another one. In certain cases it may be necessary to use a special style
for the tab content wrapper element, for example to remove the padding
from the wrapper and left the tab content element itself to handle it.
To support that now the tab content wrapper has the "tab-{tabId}" CSS
class in addition to the "tab" CSS class.

Another option would have been to make possible to specify the class
name to use when a tab was added so the same class could be used by the
wrapper when showing different tabs. However this seem like an uncommon
use case, and there should not be a lot of tabs anyway, so the current
approach would not require a lot of ".tab-{tabId1}", ".tab-{tabId2}",
".tab-{tabId3}"... selectors for the same rules in the CSS files anyway.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The positioning context (the element that an absolute element is
relative to) of the tooltips shown for the date of messages was the
author row, and thus they were visible only were the author row would be
visible. The author row would be clipped outside the list of messages
(due to its "overflow-y" CSS property), so the tooltip shown for the
date of the newest message was clipped at the top of the list of
messages.

Now the tooltips are added to the chat view itself, so "#app-content"
becomes their positioning context and thus are no longer clipped by the
list of messages.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Limiting the scroll bar in the sidebar to the list of chat messages
causes the scroll bar to be removed from the whole sidebar in other tabs
too. Therefore, the scroll bars must be explicitly enabled in the other
tab contents that need them.

The list of participants grows dynamically, so a vertical scroll bar
should be enabled on it to be able to view all the participants in a
long list.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the limit-scroll-bar-in-sidebar-to-chatview branch from 7e9805b to bccc62c Compare January 9, 2018 23:45

@Ivansss Ivansss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@Ivansss Ivansss merged commit 290b00b into master Jan 10, 2018
@Ivansss Ivansss deleted the limit-scroll-bar-in-sidebar-to-chatview branch January 10, 2018 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: chat 💬 Chat and system messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants