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(patchers): fix importing images and sounds using @nextcloud/router #142

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Apr 14, 2023

☑️ Resolves

🖼️ Screenshots

No visual changes in the current version

🚧 Tasks

Previously all the files were requested from the server, except sounds - they were imported as a local file from a copy. This is a problem because:

  • Talk images should also be imported from the local files, not requested

  • Talk sounds should not be copied to this repo - we can require them from the spreed repo during the build. Only the notifications app's sounds could be copied.

  • Update generateFilePath patch from @nextcloud/router:

    • Always require images and sounds for 'spreed' app from the spreed repository
    • Require sounds only for the 'notifications' app from the local copy
    • Remove unused copy of the spreed's sounds
  • Patch imagePath from @nextcloud/router to use patched generateFilePath

@ShGKme ShGKme added the bug Something isn't working label Apr 14, 2023
@ShGKme ShGKme added this to the v0.3.1 milestone Apr 14, 2023
@ShGKme ShGKme requested a review from Antreesy April 14, 2023 08:59
@ShGKme ShGKme changed the title fix(patchers): fix importing images using @nextcloud/router fix(patchers): fix importing images and sounds using @nextcloud/router Apr 14, 2023
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 at mentioned PR.
Works properly!

src/patchers/nextcloud-router.js Outdated Show resolved Hide resolved
@ShGKme ShGKme modified the milestones: v0.3.1, v0.3.2 Apr 16, 2023
@ShGKme ShGKme requested a review from Antreesy April 19, 2023 10:08
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, still works!

Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
@ShGKme ShGKme force-pushed the fix/noid/fix-img-import-with-nextcloud-router branch from bcf6545 to 0b3a512 Compare April 19, 2023 13:40
@ShGKme
Copy link
Contributor Author

ShGKme commented Apr 19, 2023

Rebased onto main and rebase --autosquash, no changes

@ShGKme ShGKme merged commit 52ab187 into main Apr 19, 2023
@delete-merged-branch delete-merged-branch bot deleted the fix/noid/fix-img-import-with-nextcloud-router branch April 19, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants