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

feat: use local server styles instead of requested #741

Merged
merged 5 commits into from
Aug 3, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Aug 3, 2024

☑️ Resolves

Currently Talk Desktop has copy of master server styles used on the Login and then request remote server styles from the connected Nextcloud server.

Problems:

  • Talk frontend in the lastest version might not be compatible with old Nextcloud server
  • Loading Talk window is slower, waiting for the styles request

Proposal:

  • Store all styles locally
  • Add an util to get the styles
  • To make it compatible with different server versions - store them separately

New indirectly added features:

  • All windows no support the dark theme

Drawbacks:

  • Custom themes, including primary color and background, are not supported anymore :(

🖼️ Screenshots

🏚️ Before 🏡 After
image image
before-load after-load

🚧 Tasks

  • Add script to fetch server styles for a specific Server version
    • Copies core styles and files
    • Fetches PHP generated server styles from themes app
    • Adjusts .reuse/dep5 for fetched styles
  • During the build, get the styles depending on used Talk version
  • Import local styles in app instead of making the request
  • Adjust styles a bit

@ShGKme ShGKme self-assigned this Aug 3, 2024
@ShGKme ShGKme requested review from DorraJaouad and Antreesy August 3, 2024 17:24
@ShGKme ShGKme marked this pull request as ready for review August 3, 2024 17:24
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.

Tested with linux package. Feels a lot, but make sense to store the styles locally

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 3, 2024

Note: currently it doesn't support stable29 and below, because it relies on .reuse/dep5 introduced in Nextcloud 30. but we aren't planning to built Talk Desktop with 29 anymore

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 3, 2024

It's only the first stage on updating styles workflow.
Next steps:

  • automatically request switch and request required styles on build
  • improve fetching script to make it faster and reusable

Merging for now, seems to work. Huge change is only around generated files.

@ShGKme ShGKme merged commit 0d2976d into main Aug 3, 2024
7 checks passed
@ShGKme ShGKme deleted the fix/get-styles-via-script-and-store-localy branch August 3, 2024 21:05
@ShGKme ShGKme mentioned this pull request Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants