nextcloud / spreed

🗨️ Nextcloud Talk – chat, video & audio calls for Nextcloud
https://nextcloud.com/talk
GNU Affero General Public License v3.0
1.61k stars 429 forks source link

*Close sidebar* focused and visible after page load on mobile #10908

Closed ChristophWurst closed 10 months ago

ChristophWurst commented 10 months ago

How to use GitHub


Steps to reproduce

  1. Open Talk on mobile browser

Expected behaviour

I see the conversation list or chat distraction free

Actual behaviour

image

This is probably an accessibility feature from nextcloud/vue. Maybe the focus after page load should be set elsewhere?

Talk app

Talk app version: (see apps admin page: /index.php/settings/apps)

Custom Signaling server configured: yes/no and version (see additional admin settings: /index.php/index.php/settings/admin/talk#signaling_server)

Custom TURN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#turn_server)

Custom STUN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#stun_server)

Browser

Microphone available: yes/no

Camera available: yes/no

Operating system: Windows/Ubuntu/...

Browser name: Firefox/Chrome/...

Browser version: 85/96/...

Browser log

``` Insert your browser log here, this could for example include: a) The javascript console log b) The network log c) ... ```

Server configuration

Operating system: Ubuntu/RedHat/...

Web server: Apache/Nginx

Database: MySQL/Maria/SQLite/PostgreSQL

PHP version: 8.0/8.1/8.2

Nextcloud Version: (see admin page)

List of activated apps:

``` If you have access to your command line run e.g.: sudo -u www-data php occ app:list from within your server installation folder ```

Nextcloud configuration:

``` If you have access to your command line run e.g.: sudo -u www-data php occ config:list system from within your Nextcloud installation folder ```

Server log (data/nextcloud.log)

``` Insert your server log here ```
Antreesy commented 10 months ago

Yep, came with this PR: https://github.com/nextcloud-libraries/nextcloud-vue/pull/4633

Hopefully will be fixed by this PR: https://github.com/nextcloud-libraries/nextcloud-vue/pull/4864

Focused search input on page load should be fine, i guess?

ShGKme commented 10 months ago

@nextcloud/designers Do you think we should have the sidebar initially opened on mobile by default? What about:

It is always annoying for me when I open a chat in a small window, because I need to close the sidebar first

ChristophWurst commented 10 months ago

Focused search input on page load should be fine, i guess?

On mobile this might cause the keyboard to show. So to add to the problem @ShGKme outlined above, you would have to close keyboard and sidebar to see the main content.

marcoambrosini commented 10 months ago

agree @ShGKme. I actually wouldn't even make it configurable. Do you have any use case in mind where the sidebar should be open when navigating to the page?

ShGKme commented 10 months ago

Do you have any use case in mind where the sidebar should be open when navigating to the page?

No. That's just how it has been working all the time :D (Since you created the mobile view of the sidebar) https://github.com/nextcloud-libraries/nextcloud-vue/pull/989

marcoambrosini commented 10 months ago

Sorry, I misinterpreted the issue. I thought we were talking about the right sidebar. Indeed, we need to have the left sidebar open only when the URL ends with /apps/spreed/

szaimen commented 10 months ago

Indeed, the sidebar should be closed by default on mobile. Apps are already able to overwrite this on their own. See e.g. https://github.com/nextcloud/spreed/blob/47ea02cbaaf335a6c6585d01e524394b7ea2ef87/src/App.vue#L488-L490

Antreesy commented 10 months ago

Also when focusing the search box, we're facing a loop for mobiles / desktops with small screen resolution:

https://github.com/nextcloud/spreed/assets/93392545/1223aad1-8e81-488b-a1aa-a9975978d933

Workaround I have found so far is not showing the trailing button unless there is a non-empty value (reverts changes asked by @marcoambrosini https://github.com/nextcloud/spreed/issues/9974)

marcoambrosini commented 10 months ago

@Antreesy as a fix, let's make the trailing button non-focusable tabindex="-1" if value === '' And in order for everyone to benefit from this and other perks of the search functionality, we should probably upstream this into a NcSearchInput component.

nickvergessen commented 10 months ago
Antreesy commented 10 months ago

let's make the trailing button non-focusable

There can't be enough direct DOM manipulations, but yeah, it does the job

ShGKme commented 10 months ago

Fixed by: