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

DRY up image uploading code #2477

Merged
merged 4 commits into from
Dec 1, 2020
Merged

DRY up image uploading code #2477

merged 4 commits into from
Dec 1, 2020

Conversation

nina-py
Copy link
Contributor

@nina-py nina-py commented Nov 30, 2020

Refs #1306

Changes proposed in this pull request:

Abstracted common functionality into a generic UploadImageController class. Did not tackle SVG support as it's not defined in the issue what this should be - simply leave them as is as for .ico files? Or something else? Happy to tackle this either in this PR or separately.

Reviewers should focus on:

The refactored code; new class property names in particular in UploadImageController.

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

Abstracted common functionality into a generic UploadImageController class.
Copy link
Sponsor Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Looks great, thank you very much! A few minor style comments:

src/Api/Controller/UploadLogoController.php Outdated Show resolved Hide resolved
src/Api/Controller/UploadFaviconController.php Outdated Show resolved Hide resolved
@askvortsov1
Copy link
Sponsor Member

Confirmed to work locally, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants