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.

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

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

Closed
ChristophWurst opened this issue Nov 15, 2023 · 12 comments · Fixed by #11065
Closed

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

ChristophWurst opened this issue Nov 15, 2023 · 12 comments · Fixed by #11065

Comments

@ChristophWurst
Copy link
Member

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

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
Copy link
Contributor

Antreesy commented Nov 21, 2023

Yep, came with this PR: nextcloud-libraries/nextcloud-vue#4633

Hopefully will be fixed by this PR: nextcloud-libraries/nextcloud-vue#4864

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

@ShGKme
Copy link
Contributor

ShGKme commented Nov 22, 2023

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

  • Make it closed by default
  • Make it configurable, so any app can decide

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

@ChristophWurst
Copy link
Member Author

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
Copy link
Member

marcoambrosini commented Nov 22, 2023

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
Copy link
Contributor

ShGKme commented Nov 22, 2023

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)
nextcloud-libraries/nextcloud-vue#989

@marcoambrosini
Copy link
Member

marcoambrosini commented Nov 22, 2023

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
Copy link
Contributor

szaimen commented Nov 22, 2023

Indeed, the sidebar should be closed by default on mobile. Apps are already able to overwrite this on their own. See e.g.

spreed/src/App.vue

Lines 488 to 490 in 47ea02c

emit('toggle-navigation', {
open: true,
})

@Antreesy
Copy link
Contributor

Antreesy commented Nov 22, 2023

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

  • Tabbing from input moves the focus to the trailing button
  • At the nextTick, trailing button will be deleted, as system assumes search field to be blurred
  • No element to focus => focus-trap falls back to the first available element => input field again
loop.mp4

Workaround I have found so far is not showing the trailing button unless there is a non-empty value (reverts changes asked by @marcoambrosini #9974)

@marcoambrosini
Copy link
Member

@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
Copy link
Member

nickvergessen commented Nov 23, 2023

@Antreesy
Copy link
Contributor

let's make the trailing button non-focusable

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants