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

[2.x] Custom Markdown heading renderer #2047

Merged
merged 100 commits into from
Dec 5, 2024

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Dec 1, 2024

Abstract

Changes

  • Removed auto-configuration for HeadingPermalinkExtension, meaning user added custom options provided to that extension will be ignored. If you for some reason require that extension's custom features, either implement those to the Blade component after publishing it, or disable the Hyde permalink feature from the configuration and add the extension yourself.
  • Removes the MarkdownService withPermalinks and canEnablePermalinks methods, see comment
  • [2.x] Semantic Tailwind Markdown heading permalinks #2052

@caendesilva
Copy link
Member Author

Cursor (Claude) analysis of the three approaches for implementing heading permalinks:

1. Original CommonMark Extension Implementation

    protected function configurePermalinksExtension(): void
    {
        $this->addExtension(HeadingPermalinkExtension::class);
                'fragment_prefix' => '',
        $this->config = array_merge([
            'heading_permalink' => [
                'id_prefix' => '',
                'fragment_prefix' => '',
                'symbol' => '',
                'insert' => 'after',
                'min_heading_level' => 2,
                'aria_hidden' => false,
            ],
        ], $this->config);
    }

Benefits:

  • Simple to implement and maintain
  • Follows CommonMark's standard extension pattern
  • Performant since it's handled during Markdown parsing
  • Consistent behavior across all Markdown content

Drawbacks:

  • Limited customization options
  • Requires modifying CommonMark configuration
  • Can't easily change permalink position/style per heading
  • Tightly coupled to CommonMark implementation

2. Full Blade Component Replacement

@props(['level' => 1, 'id' => null])

@php
$tag = 'h' . $level;
$id = $id ?? \Illuminate\Support\Str::slug($slot);
@endphp

<{{ $tag }} {{ $attributes->merge(['id' => $id]) }}>
    {{ $slot }}
    @if(config('markdown.features.permalinks', true))
        <a href="#{{ $id }}" class="heading-permalink" aria-label="Permalink for this section"></a>
    @endif
</{{ $tag }}> 

Benefits:

  • Full control over heading HTML structure
  • Can customize behavior per heading instance
  • Easy to modify styling and structure
  • Can add additional features like custom icons
  • More maintainable Blade templating

Drawbacks:

  • More complex implementation
  • Requires replacing CommonMark's heading renderer
  • Slightly higher performance overhead
  • Need to maintain heading level logic
  • Must handle ID generation manually

3. Permalink-Only Component Injection

Example implementation:

// components/heading-permalink.blade.php
@props(['id'])

<a href="#{{ $id }}" 
   class="heading-permalink" 
   aria-label="Permalink for this section">
</a>

Benefits:

  • Minimal implementation
  • Easy to inject before/after existing headings
  • Keeps CommonMark's heading rendering intact
  • Very focused responsibility
  • Lightweight performance impact

Drawbacks:

  • Less control over heading structure
  • Need to coordinate with existing heading HTML
  • May require additional DOM manipulation
  • Could have positioning challenges
  • Limited contextual awareness

Recommendation

I recommend using the full Blade component replacement approach (#2) because:

  1. It provides the most flexibility for future customization
  2. Keeps the implementation in pure Blade/PHP without CommonMark dependencies
  3. Allows per-heading customization when needed
  4. Easier to maintain and modify
  5. Better integration with Laravel/Blade ecosystem
  6. Can still use existing CSS without changes:
.prose h1, .prose h2, .prose h3, .prose h4, .prose h5, .prose h6 {
    @apply w-fit;
}

.prose :is(h1,h2,h3,h4,h5,h6):hover .heading-permalink,
.prose :is(h1,h2,h3,h4,h5,h6):focus .heading-permalink {
    @apply opacity-75 grayscale transition-opacity duration-100 ease-out;
}

.heading-permalink {
    @apply opacity-0 ml-1 transition-opacity duration-300 ease-linear px-1 scroll-m-4;
}

.heading-permalink::before {
    @apply content-['#'];
}

.heading-permalink:hover, .heading-permalink:focus {
    @apply opacity-100 grayscale-0;
}

The slight performance overhead is negligible for most sites, and the benefits of having full control over the heading structure outweigh the drawbacks. It also makes it easier to add features like custom icons, positioning options, and accessibility improvements in the future.

The permalink-only component approach might seem simpler at first, but it introduces complexity in positioning and coordination with existing heading elements. The full component replacement gives us a clean slate to work with while maintaining all the functionality we need.

@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from 61386bd to e1ccda8 Compare December 1, 2024 18:04
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (166a551) to head (3df7aa2).
Report is 101 commits behind head on new-asset-system.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             new-asset-system     #2047   +/-   ##
====================================================
  Coverage              100.00%   100.00%           
- Complexity               1925      1931    +6     
====================================================
  Files                     195       196    +1     
  Lines                    5122      5138   +16     
====================================================
+ Hits                     5122      5138   +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caendesilva caendesilva changed the title [2.x] Custom markdown heading renderer [2.x] Custom Markdown heading renderer Dec 1, 2024
@caendesilva caendesilva mentioned this pull request Dec 1, 2024
17 tasks
@caendesilva
Copy link
Member Author

caendesilva commented Dec 5, 2024

Edge case we need to fix: If there are multiple headings of the same name, we need offset. (Done)

@caendesilva
Copy link
Member Author

Was wondering if we should have a config option for the hash symbol. Until users request it I think I'm going to hold off on that. Claude's (via Cursor) explanation below is very in line with my thoughts.


Based on the context and the nature of this customization, I recommend requiring users to publish the blade component rather than adding a configuration option. Here's why:

  1. The hash symbol (#) is a widely recognized standard for permalink/anchor links across the web
  2. Looking at the configuration files, particularly config/markdown.php, we can see the philosophy is to keep configuration options focused on essential functionality rather than visual customizations
  3. The existing documentation (as seen in docs/digging-deeper/customization.md) already guides users on how to publish and customize Blade views when needed
  4. This aligns with Laravel's approach where visual/UI customizations are typically handled through view publishing rather than configuration

If someone wants to customize the permalink icon, they can:

  1. Run php hyde publish:views
  2. Edit the published component in resources/views/vendor/hyde/components/markdown-heading.blade.php
  3. Change the "#" to their desired icon/text

This approach:

  • Keeps the configuration clean and focused on core functionality
  • Provides more flexibility since users can add HTML/CSS classes along with changing the icon
  • Follows the established pattern for UI customizations in Hyde/Laravel
  • Avoids adding rarely-used configuration options

The documentation already shows this is the recommended approach for UI customizations, as seen in:

### Customizing the Blade templates

To make it really easy to customize asset loading, the styles and scripts are loaded in dedicated Blade components.

- Styles are loaded in `hyde::layouts.styles`
- Scripts are loaded in `hyde::layouts.scripts`

To customize them, run the following command:

```bash
php hyde publish:views layouts
```

Then edit the files found in `resources/views/vendor/hyde/layouts` directory of your project.

@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from 4e378fd to 8282ccd Compare December 5, 2024 10:29
@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from 8282ccd to 3feddd3 Compare December 5, 2024 10:32
@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from b9ae134 to 9948afd Compare December 5, 2024 18:40
@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from dcd2b03 to c4e67ff Compare December 5, 2024 18:52
@caendesilva
Copy link
Member Author

Just for absolute fun, not that I thought this would in any way be a performance sink, but the code that generates unique identifiers takes only 0.011ms for a heading registry with 1 000 000 items.

@caendesilva caendesilva force-pushed the custom-markdown-heading-renderer branch from cfac7e7 to 3df7aa2 Compare December 5, 2024 19:14
@caendesilva caendesilva merged commit 62f5092 into new-asset-system Dec 5, 2024
12 checks passed
@caendesilva caendesilva deleted the custom-markdown-heading-renderer branch December 5, 2024 20:15
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.

2 participants