-
Notifications
You must be signed in to change notification settings - Fork 508
Add chat tab to sidebar in Files app #2611
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.
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
Conversation
marcoambrosini
left a comment
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.
Works nicely, just a couple of minor things 👍
src/FilesSidebarTabApp.vue
Outdated
|
|
||
| <script> | ||
| // Explicitly enable smart-tabs so it is also applied to the CSS rules. | ||
| /* eslint no-mixed-spaces-and-tabs: ["error", "smart-tabs"] */ |
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.
| /* eslint no-mixed-spaces-and-tabs: ["error", "smart-tabs"] */ |
No hiding please, the coding style is the same across all nextcloud vue apps
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.
It is not hiding anything, just enabling smart tabs so the comments can use spaces after the tabs for proper alignment (it works by default in the JavaScript code, but I have no idea why it does not in the CSS code). That is, to be able to write
/* The chat view shares its parent with the call button, so the default
* "height: 100%" needs to be unset. */
instead of
/* The chat view shares its parent with the call button, so the default
* "height: 100%" needs to be unset. */
But as I said I would expect that to work too for the CSS code by default, so I do not know what I am missing.
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.
Well then this should be added to https://github.com/nextcloud/eslint-config-nextcloud/blob/master/index.js
This is just a temporary message; in the future the whole tab will be hidden in that case, but it depends on a fix in server which was not ready yet for beta 3. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Marco Ambrosini <[email protected]>
The Talk sidebar is supported for a file when it meets certain sharing conditions. If the Talk sidebar is not supported then a message is shown to the user in the tab to share the file first. In some cases it is not possible to know if the Talk sidebar is supported or not only from the FileInfo data and it is necessary to query the server. In those cases a loading icon is shown until the response from the server is received. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When the Talk sidebar is supported for a file a message is shown to the user to explicitly join the conversation, and once joined the chat view is shown in the chat tab. If a different file is selected the previous conversation is left (even if the conversation for the new file is not joined). The sidebar tabs are created again whenever the sidebar is opened, be it when changing to a different file or even when opening the sidebar again for the same file after closing it. As the file of the current conversation needs to be known to leave the conversation or not when the FileInfo changes (so it is left if it is opened in a different file, but kept if opened in the same file) the file ID of the current conversation needs to be stored outside of the view. Finally, as the AdvancedInput in shown in the chat view but there is no route in use "this.$route.name" needs to be guarded. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Before, when the chat view was shown in the sidebar the sidebar height grew to accomodate the full list of messages (or the chat view just filled a section of the sidebar, depending on the browser). Instead of that the chat view should grow to fill the available space in the tab content, and the scroll bar should be provided by the list of messages itself. In order to do that it is necessary to modify the style of the tabs content container to set "overflow: hidden". In Nextcloud 17 it was possible to provide an additional set of CSS classes to be set in the tabs content container when a tab was active with "getTabsContainerExtraClasses", but unfortunately that is not currently possible in Nextcloud 18. Therefore this needs to be done by explicitly setting the desired CSS rules on the tabs content container when the chat tab is the active tab (and removing it when it is no longer the active tab). Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
1bb87e7 to
eb7aa7b
Compare
|
Open tasks moved to #2619 |
Known issues: