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

fix(viewer): improve styling and loading/error handling #783

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Sep 3, 2024

☑️ Resolves

  • Media is sometimes cut on the bottom
  • On slow connection there is just an empty modal
  • No error handling

🚧 Tasks

  • refactor(viewer): make viewer initialized async
    • required to use @nextcloud/files with @nextcloud/router because it uses router on module init, which is not ready until Talk Desktop is ready
  • refactor(viewer): move path utils to a module
  • fix(viewer): media is cut on the bottom sometimes
    • NcModal styles in fullscreen has hardcoded position and it wasn't correctly changed to support window title bar
  • fix(viewer): add loading and error handling with improved UI
    • Add some new universal components for viewer
      • Base - to display content/loading/error
      • Media - specifically for media to handle media loading and styles

🖼️ Screenshots

🏚️ Before 🏡 After
Small and large Windows .
image image
image image
Error .
image image
Slow connection .
image image

Notes about loading

On fast connection there is no spinner to not blink. It only appears after --animation-slow delay.

Video is also shown once it's ready to be played.

Slow Fast
slow fast

Required to prepare packages (`@nextcloud/router`) used in `@nextcloud/files`

Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme added bug Something isn't working 3. to review labels Sep 3, 2024
@ShGKme ShGKme self-assigned this Sep 3, 2024
@ShGKme ShGKme mentioned this pull request Sep 3, 2024
2 tasks
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Looks nice and smooth now =)

@ShGKme
Copy link
Contributor Author

ShGKme commented Sep 4, 2024

Thanks @DorraJaouad and @Antreesy, I forgot to test on the light theme 🌕

Fixed, see the last commit

Copy link

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Tested 🦅 🦅

@ShGKme
Copy link
Contributor Author

ShGKme commented Sep 4, 2024

Squashed

@ShGKme ShGKme force-pushed the fix/viewer-style-and-loading-error-handling branch from 43068be to 5348be3 Compare September 4, 2024 13:08
@ShGKme ShGKme enabled auto-merge September 4, 2024 13:08
@ShGKme ShGKme merged commit 6b6f6a2 into main Sep 4, 2024
7 checks passed
@ShGKme ShGKme deleted the fix/viewer-style-and-loading-error-handling branch September 4, 2024 13:09
@ShGKme ShGKme mentioned this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants