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.

Sign up for GitHub

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

Jump to bottom

Remove redundancy and merge Meta and Metadata models #384 #390

Merged
merged 25 commits into from
May 18, 2022
Merged

Remove redundancy and merge Meta and Metadata models #384 #390

merged 25 commits into from
May 18, 2022

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented May 17, 2022 •
edited
Loading

Refactors the internal API to reduce redundancy and code overlap.

Also set canonical meta tags when a site base URL is set, fixes #376

Fixes bug where blog post og:url did not contain the .html extension even when pretty URLs were disabled.

New metadata handling

Internally changes the way metadata is handled. Previously global config defined tags were added in the Blade views at build time, but PageModel specific tags were generated when the object was constructed. I don't think that workflow makes sense, since the metadata is only ever used when compiling the Blade view, we don't benefit from constructing it together with the object model. Instead, all the meta tags for the head section are generated during build through the $page property. This then takes care of merging dynamic tags with priority over the global config defined ones.

Before:

// In meta.blade.php
{{-- Config Defined Tags --}}
@foreach (config('hyde.meta', []) as $name => $content) 
<meta name="{{ $name }}" content="{{ $content }}">
@endforeach

{{-- Add any extra tags to include in the <head> section --}}
<meta property="og:site_name" content="{{ config('hyde.name', 'HydePHP') }}">

{{-- Add any extra meta tags to include after the others --}}
@stack('meta')

// In post.blade.php
@push('meta')
<!-- Blog Post Meta Tags -->
@foreach ($page->getMetadata() as $name => $content)
    <meta name="{{ $name }}" content="{{ $content }}">
@endforeach
@foreach ($page->getMetaProperties() as $name => $content)
    <meta property="{{ $name }}" content="{{ $content }}">
@endforeach
@endpush

After

// In meta.blade.php
{{-- Render the config defined and dynamic page meta tags --}}
{!! $page->renderPageMetadata() !!}

{{-- Add any extra tags to include in the <head> section --}}
@stack('meta')

// The post meta tags are already merged and inserted into the rendePageMetadata() helper

@caendesilva caendesilva linked an issue May 17, 2022 that may be closed by this pull request
Remove redundancy and merge Meta and Metadata models #384
Closed
3 tasks
caendesilva and others added 17 commits May 18, 2022 12:01
@caendesilva
Extract metadata helpers to concern
72b1356
@caendesilva
Reset config state in setUp method
94aa138
@StyleCIBot
Apply fixes from StyleCI
6676f31
Merge pull request #391 from hydephp/analysis-BMOk77
0cfb82b 
Apply fixes from StyleCI

[ci skip] [skip ci]
@caendesilva
Merge legacy metadata model with new helper
b4793d1
@caendesilva
Rename methods to be more descriptive
06f909a
@caendesilva
Fix compatibility
92a2c58
@StyleCIBot
Apply fixes from StyleCI
0113160
Merge pull request #392 from hydephp/analysis-YjVv3d
e3620de 
Apply fixes from StyleCI

[ci skip] [skip ci]
@caendesilva
Rename HasMetadata to GeneratesPageMetadata
5e910e0
@caendesilva
Update code for trait rename
c69bf77
@caendesilva
Fix failing test caused by missing $page property
4103707
@caendesilva
Rename test for renamed trait
4db7571
@caendesilva
Fix merge algorithm to give precedence to supplied overrides
02d4e41
@caendesilva
Meta tags are now merged at runtime
cf44a62
@StyleCIBot
Apply fixes from StyleCI
cf80245
Merge pull request #393 from hydephp/analysis-wjGP79
6b114ed 
Apply fixes from StyleCI

[ci skip] [skip ci]
@caendesilva caendesilva marked this pull request as ready for review May 18, 2022 11:09
@caendesilva caendesilva merged commit f15d493 into master May 18, 2022
@caendesilva caendesilva deleted the 384-merge-meta-and-metadata-models branch May 18, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove redundancy and merge Meta and Metadata models Set canonical meta tags when a site base URL is set
2 participants
@caendesilva @StyleCIBot