Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

ImageRule #78

Closed
ghost opened this issue Apr 25, 2016 · 5 comments
Closed

ImageRule #78

ghost opened this issue Apr 25, 2016 · 5 comments
Labels

Comments

@ghost
Copy link

ghost commented Apr 25, 2016

Is it possible to have the presentation property parsed in the image rule, the same way it is done in the VideoRule?

E.G.

    if ($this->getProperty(Video::ASPECT_FIT, $node)) {
        $video->withPresentation(Video::ASPECT_FIT);
    } elseif ($this->getProperty(Video::ASPECT_FIT_ONLY, $node)) {
        $video->withPresentation(Video::ASPECT_FIT_ONLY);
    } elseif ($this->getProperty(Video::FULLSCREEN, $node)) {
        $video->withPresentation(Video::FULLSCREEN);
    } elseif ($this->getProperty(Video::NON_INTERACTIVE, $node)) {
        $video->withPresentation(Video::NON_INTERACTIVE);
    }
@marianop
Copy link

Hi! I'm going to expand on this.

In the Image Element this properties are set:

const ASPECT_FIT = 'aspect-fit';
const ASPECT_FIT_ONLY = 'aspect-fit-only';
const FULLSCREEN = 'fullscreen';
const NON_INTERACTIVE = 'non-interactive';

The same way they are on the Video Element.
Even there is the withPresentation function, that has been duplicated from the Video Elemento because the comment has the Video on it:

/
**
     * Sets the aspect ration presentation for the video.
     *
     * @param string $presentation one of the constants ASPECT_FIT, ASPECT_FIT_ONLY, FULLSCREEN or NON_INTERACTIVE
     * @see Image::ASPECT_FIT
     * @see Image::ASPECT_FIT_ONLY
     * @see Image::FULLSCREEN
     * @see Image::NON_INTERACTIVE
     *
     * @return $this
     */
    public function withPresentation($presentation)
    {
        Type::enforceWithin(
            $presentation,
            [
                Image::ASPECT_FIT,
                Image::ASPECT_FIT_ONLY,
                Image::FULLSCREEN,
                Image::NON_INTERACTIVE
            ]
        );
        $this->presentation = $presentation;

        return $this;
    }

But in the ImageRule Transformer, the rules for the properties are missing, specifically this:

if ($this->getProperty(Image::ASPECT_FIT, $node)) {
            $image->withPresentation(Image::ASPECT_FIT);
        } elseif ($this->getProperty(Image::ASPECT_FIT_ONLY, $node)) {
            $image->withPresentation(Image::ASPECT_FIT_ONLY);
        } elseif ($this->getProperty(Image::FULLSCREEN, $node)) {
            $image->withPresentation(Image::FULLSCREEN);
        } elseif ($this->getProperty(Image::NON_INTERACTIVE, $node)) {
            $image->withPresentation(Image::NON_INTERACTIVE);
        }

I added them and worked perfectly.

@everton-rosario
Copy link
Contributor

@marianop Can you send a PR with that fix?

@marianop
Copy link

I tried, but I'm not an approved contributor so I can't make PRs :(

@everton-rosario
Copy link
Contributor

@marianop What do you mean?
If you follow the steps here, you will be promoted as a contributor:
https://github.com/facebook/facebook-instant-articles-sdk-php/blob/master/CONTRIBUTING.md

Let me know if you have any problem on these steps.

@everton-rosario
Copy link
Contributor

Fixed in PR #147

everton-rosario added a commit that referenced this issue Jul 15, 2016
Fixes #78 adding support to presentation in Image element
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants