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 regression for template extensions ( A beta 16 breaking change) #281

Closed
eddiewebb opened this issue Apr 25, 2021 · 1 comment · Fixed by #286
Closed

fix regression for template extensions ( A beta 16 breaking change) #281

eddiewebb opened this issue Apr 25, 2021 · 1 comment · Fixed by #286
Labels

Comments

@eddiewebb
Copy link

Bug from change

Bug Report

Current Behavior
Prior to beta 16 my extension (gpx preview) added a custom template to upload, worked great.

changes as part of beta 16 introduced hard-coded switch statement inclusive only of your 4 templates. And now my plugin is broke.

Steps to Reproduce

  1. create extension with a "tag" not in the predefined switch
  2. it defaults to a file URL

Expected Behavior
It looks up tag based on the list of templates it has, and then asks the tag'd template for it's appropriate BBcode.

Screenshots

export default function fileToBBcode(file) {
    switch (file.tag()) {
        // File
        case 'file':
            return `[upl-file uuid=${file.uuid()} size=${file.humanSize()}]${file.baseName()}[/upl-file]`;

        // Image template
        case 'image':
            return `[upl-image uuid=${file.uuid()} size=${file.humanSize()} url=${file.url()}]${file.baseName()}[/upl-image]`;

        // Image preview
        case 'image-preview':
            return `[upl-image-preview url=${file.url()}]`;

        // 'just-url' or unknown
        default:
            return file.url();
    }
}

Environment

  • Flarum version: 0.0.1-beta16
  • Extension version: 0.14.0

Possible solution(s)
A hard coded switch like this is generally a sign that some logic should be delegated to the child/component classes. We already have a list of all registered templates, and a lookup function.

We should shift interface of templates to (i think logically) include the BBCode (oh wait, they do!) they generate/ parse as well. This could be done in PHP where the collection of templates lived, and a "bbcode" attribute returned from the API on upload success for the frontend to use when placing the (also server generated) url into editor.

Additional Context
I hacked the File class and overwrote with myown, fixed upload but display worked, so I think there weee other changes in a similar assumption of upload-only templates.

@eddiewebb eddiewebb added the bug label Apr 25, 2021
@eddiewebb eddiewebb changed the title Allow template extensions ( A beta 16 breaking change) fix regression for template extensions ( A beta 16 breaking change) Apr 25, 2021
@imorland imorland linked a pull request May 25, 2021 that will close this issue
3 tasks
@imorland
Copy link
Member

Hi @eddiewebb

I'm so sorry this crept in to the beta 16 update. I did spot it in the PR originally, but seems it never got addressed before merging.

I've started to work on a possible solution to this here #286 - it's still a work-in-progress, but I'd be greatful for your input, especially as your own extension is extending fof/upload templates :)

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

Successfully merging a pull request may close this issue.

2 participants