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

Set Facebook App ID as optional #4

Closed
wilderamorim opened this issue Jul 7, 2019 · 2 comments
Closed

Set Facebook App ID as optional #4

wilderamorim opened this issue Jul 7, 2019 · 2 comments

Comments

@wilderamorim
Copy link
Contributor

Not always when we are developing a website it has an App ID and even in production can happen not to have.

So I suggest you leave fb: app_id receive null value, so we do not have to leave a random value like 9999999999999999 to print

<meta property="fb:app_id" content="9999999999999999"/>

both in development and in production if that's the case, because if I do not set a value for Facebook App ID it causes me the following error:

Warning: Invalid argument supplied for foreach() in C:\xampp\htdocs\project-folder\vendor\coffeecode\optimizer\src\Optimizer.php on line 134

@machadomatt
Copy link

machadomatt commented Feb 16, 2020

Warning: Invalid argument supplied for foreach() in C:\xampp\htdocs\project-folder\vendor\coffeecode\optimizer\src\Optimizer.php on line 134

Actually, this occurs because the Facebook function isn't checking the $admins argument for an array and its values before iterate.

Optimizer.php class file with fixes:

public function facebook(string $appId = null, array $admins = null): Optimizer
    {
        if ($appId) {
            $fb = $this->meta->addChild("meta");
            $fb->addAttribute("property", "fb:app_id");
            $fb->addAttribute("content", $appId);

            return $this;
        }

        if (is_array($admins) && !empty($admins)) {
            foreach ($admins as $admin) {
                $fb = $this->meta->addChild("meta");
                $fb->addAttribute("property", "fb:admins");
                $fb->addAttribute("content", $admin);
            }
        }

        return $this;
    }

I'll try to make a PR soon.

@robsonvleite
Copy link
Owner

O problema aqui foi confundir os dados. Observando o método você deve informar um ou outro parâmetro e respeitar seus formatos.

->facebook("23434234") para APPID
OU
->facebook(null, ['394723943']) para 1 admin
OU
->facebook(null, ['394723943', ''219873892']) para admins

Coloquei uma verificação a mais no admins para validar o correto formato sem o erro. Agora se ocorrer o componente vai omitir a informação.

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

No branches or pull requests

3 participants