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.

Sign up for GitHub

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

Jump to bottom

NcAppDetailsToggle missing with viewport width 1024px #3758

Closed
mejo- opened this issue Feb 15, 2023 · 2 comments · Fixed by #3768
Closed

NcAppDetailsToggle missing with viewport width 1024px #3758

mejo- opened this issue Feb 15, 2023 · 2 comments · Fixed by #3768
Labels
bug Something isn't working
Milestone
7.7.0

Comments

@mejo-
Copy link
Contributor

mejo- commented Feb 15, 2023

When opening three-pane Nextcloud apps (with NcAppNavigationList and NcAppContentList) with a viewport width of 1024px, the NcAppContentList is inaccessible. It's closed due to https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/NcAppContent/NcAppContent.vue#L384-L386, but the NcAppDetailsToggle button is not displayed either for some reason.

When lowering the viewport width to 1023px, the NcAppDetailsToggle button becomes visible again.

When raising the viewport width to 1025px, NcAppContentList gets expanded again and all is good.

So the problem only exists with a viewport width of exactly 1024px.

This affects at least Mail and Collectives, possibly many other apps as well.

It has been reported originally here: nextcloud/collectives#433
@mejo- mejo- added the bug Something isn't working label Feb 15, 2023
@mejo- mejo- added this to the 7.7.0 milestone Feb 15, 2023
@mejo- mejo- changed the title AppDetailsToggle missing with viewport width 1024px NcAppDetailsToggle missing with viewport width 1024px Feb 15, 2023
@mejo-
Copy link
Contributor Author

mejo- commented Feb 15, 2023 •
edited
Loading

Ok, so the problem with a width of 1024px is:

Given that isMobile is used since quite some time and is used (sometimes with custom implementations) in different apps, I'd keep it as is and instead change NcAppContent to hide NcAppContentList starting only with 1023px width.

What do others think? @ChristophWurst @raimund-schluessler @skjnldsv @brueckner

@mejo- mejo- mentioned this issue Feb 15, 2023
Closed
mejo- added a commit that referenced this issue Feb 16, 2023
@mejo-
Don' regard 1024px width viewport as mobile
0b5e4d2 
* In IsMobileState.js we treat viewports smaller 1024px as mobile.
* In NcAppContent.vue we treat viewports starting with 1024px as mobile.

This results in weird behaviour with screens that have exactly 1024px
width. E.g., on three-pane apps, NcAppContentList is hidden but the
NcAppDetailsToggle button is not displayed yet.

This commit changes NcAppContent.vue to treat viewports smaller 1024px
as mobile, to be in sync with IsMobileState.js.

Fixes: #3758

Signed-off-by: Jonas <jonas@freesources.org>
@mejo- mejo- mentioned this issue Feb 16, 2023
Merged
@mejo- mejo- mentioned this issue Feb 16, 2023
Merged
2 tasks
mejo- added a commit that referenced this issue Feb 16, 2023
@mejo-
Don' regard 1024px width viewport as mobile
6d69022 
* In IsMobileState.js we treat viewports smaller 1024px as mobile.
* In NcAppContent.vue we treat viewports starting with 1024px as mobile.

This results in weird behaviour with screens that have exactly 1024px
width. E.g., on three-pane apps, NcAppContentList is hidden but the
NcAppDetailsToggle button is not displayed yet.

This commit changes NcAppContent.vue to treat viewports smaller 1024px
as mobile, to be in sync with IsMobileState.js.

This PR accompanies nextcloud/server#36751.

Both are required to fix
#3758.

Fixes: #3758

Signed-off-by: Jonas <jonas@freesources.org>
@mejo-
Copy link
Contributor Author

mejo- commented Feb 16, 2023

The following to PRs are required to fix this issue:

backportbot-nextcloud bot pushed a commit to nextcloud/server that referenced this issue Feb 17, 2023
@mejo- @backportbot-nextcloud
Don't regard 1024px width viewport as mobile
9d6e755 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/server that referenced this issue Feb 18, 2023
@mejo-
Don't regard 1024px width viewport as mobile
5e06e63 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/server that referenced this issue Feb 18, 2023
@mejo-
Don't regard 1024px width viewport as mobile
fa10818 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
nextcloud-command pushed a commit to nextcloud/server that referenced this issue Feb 26, 2023
@mejo- @MichaIng
Don't regard 1024px width viewport as mobile
9cf7ed4 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
RonaldBarnes pushed a commit to RonaldBarnes/NextCloud-server that referenced this issue Mar 1, 2023
@mejo- @RonaldBarnes
Don't regard 1024px width viewport as mobile
e314128 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 25, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
e38ba43 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 25, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
954b3c0 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 25, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
2ad695d 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 25, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
5acb36e 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 25, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
aaf0c79 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 26, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
b96e887 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 26, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
302bb62 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 26, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
fff0618 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 26, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
1539490 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
summersab pushed a commit to summersab/server that referenced this issue Jun 26, 2023
@mejo- @summersab
Don't regard 1024px width viewport as mobile
a3fb855 
Currently we treat 1024px as mobile in some places (e.g. `isMobile` in
nextcloud-vue) and as not mobile yet in others (e.g. in mobile.css).

This commit changes `mobile.scss` to treat viewports smaller than 1024px
as mobile.

This PR accompanies nextcloud-libraries/nextcloud-vue#3768

Both are required to fix
nextcloud-libraries/nextcloud-vue#3758.

Signed-off-by: Jonas <jonas@freesources.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
7.7.0
Development

Successfully merging a pull request may close this issue.

Don't regard 1024px width viewport as mobile nextcloud-libraries/nextcloud-vue
1 participant
@mejo-