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

Keep layout on sync errors #6903

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

max-nextcloud
Copy link
Collaborator

@max-nextcloud max-nextcloud commented Feb 14, 2025

See #6877

Ui changes

  • fix(ui): move document status to the bottom

๐Ÿ–ผ๏ธ Screenshots

๐Ÿš๏ธ before ๐Ÿก after
Sync error Bildschirmfoto vom 2025-02-14 12-56-16 grafik
Conflict view Bildschirmfoto vom 2025-02-14 00-00-36 Bildschirmfoto vom 2025-02-14 00-00-21
Notifications Bildschirmfoto vom 2025-02-14 13-19-12 Bildschirmfoto vom 2025-02-14 13-18-25

Other commits

  • fix(vite): allow hostnames used for hmr via docker-dev

โ˜‘๏ธ TODO

  • make sure that the margin from the bottom is of size --clickable-area

@max-nextcloud max-nextcloud requested a review from mejo- as a code owner February 14, 2025 11:11
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 37.86%. Comparing base (fb70ab7) to head (f14eb27).

Files with missing lines Patch % Lines
src/components/Editor.vue 0.00% 2 Missing โš ๏ธ
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6903      +/-   ##
==========================================
+ Coverage   37.84%   37.86%   +0.01%     
==========================================
  Files         750      740      -10     
  Lines       42664    42642      -22     
  Branches     1273     1263      -10     
==========================================
  Hits        16146    16146              
+ Misses      25896    25884      -12     
+ Partials      622      612      -10     

โ˜” View full report in Codecov by Sentry.
๐Ÿ“ข Have feedback on the report? Share it here.

@max-nextcloud max-nextcloud requested review from marcoambrosini and a team February 14, 2025 12:35
@max-nextcloud max-nextcloud self-assigned this Feb 14, 2025
@max-nextcloud max-nextcloud force-pushed the enh/keep-layout-on-sync-errors branch from af40cfd to d9d2506 Compare February 14, 2025 14:23
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Looks good @max-nextcloud, let's just make sure that the margin from the bottom is of size --clickable-area

@max-nextcloud max-nextcloud force-pushed the enh/keep-layout-on-sync-errors branch 2 times, most recently from bff0826 to adc0bdb Compare February 17, 2025 21:54
@max-nextcloud
Copy link
Collaborator Author

/backport to stable30

@max-nextcloud
Copy link
Collaborator Author

/backport to stable29

@max-nextcloud max-nextcloud force-pushed the enh/keep-layout-on-sync-errors branch from adc0bdb to f14eb27 Compare February 18, 2025 07:12
@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Feb 18, 2025

@marcoambrosini I noticed an issue with the layout:
Now the message container will overlay the scrollbar of the text window:
grafik

Possible solutions

Leave some room for the scrollbar

I can work around this by making the message calc(100vw - 20px) wide.

Use flex column layout on the parent

This way the message is on the bottom and the room above is for the text content.
grafik

I'm not sure which one to pick.

@marcoambrosini
Copy link
Member

marcoambrosini commented Feb 18, 2025

Did you try to play with the z-index of the element? Seems strange that it ends up above the scrollbar

@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Feb 18, 2025

Did you try to play with the z-index of the element? Seems strange that it ends up above the scrollbar

Yes. Changed it from the current 100.000 to 0 without any effect.

@marcoambrosini
Copy link
Member

@ShGKme do you have any idea why that happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ๐Ÿ‘€ In review
Development

Successfully merging this pull request may close these issues.

2 participants