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] Extract vendor file for the HydeSearch script #2031

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Nov 16, 2024

Abstract

Extracts a file in the framework vendor resources for the bulk HydeSearch scripts introduced by #2029 as it feels wrong to have so much JavaScript inline, especially as we don't really want users to tinker with it unless they know what they're doing.

Note that I decided against documenting the overloading feature as I think it's such a slim use case, and anyone determined to do it will easily be able to find it in the source.

Motivation

Benefits of this approach:

  • No need for users to run Vite/npm
  • No extra files in the skeleton project
  • JavaScript stays maintainable in its own file
  • Still works with the existing asset pipeline if users want to use Vite
  • Follows Hyde's philosophy of working out of the box

Potential concerns:

  • No minification (though impact is minimal for this small file)
  • Can't use modern JS features that need transpilation
  • Slightly less "proper" than using a proper build system

However, given Hyde's focus on simplicity and working out of the box, I think the benefits outweigh the drawbacks. We could even add a simple minified version of the file to use in production.

Customization

The search functionality can be customized by creating a resources/js/HydeSearch.js file in your project. If this file exists, Hyde will use your implementation instead of the default one.

It follows the same pattern Hyde uses for other assets and provides a clear override mechanism. Here's why it's intuitive:

  • Follows Existing Patterns: Hyde already uses this pattern for other assets and views, so it's consistent with the rest of the framework
  • Clear Override Path: Users can add their own resources/js/hyde-search.js to customize the behavior
  • Fallback Mechanism: If no custom version exists, it falls back to the vendor version
  • Zero Configuration: Works out of the box without any setup
  • Discoverable: Users familiar with Laravel/Hyde will immediately understand where to put override files

Copy link

codecov bot commented Nov 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1f4e3d4) to head (8b6ab34).
Report is 9 commits behind head on refactor-hydefront-styles-to-tailwind.

Additional details and impacted files
@@                            Coverage Diff                            @@
##             refactor-hydefront-styles-to-tailwind     #2031   +/-   ##
=========================================================================
  Coverage                                   100.00%   100.00%           
  Complexity                                    1906      1906           
=========================================================================
  Files                                          195       195           
  Lines                                         5078      5078           
=========================================================================
  Hits                                          5078      5078           

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

@caendesilva caendesilva force-pushed the recreate-the-hydesearch-plugin-with-alpine branch from 3a316af to cf4d641 Compare November 16, 2024 19:09
@caendesilva caendesilva marked this pull request as ready for review November 16, 2024 19:09
@caendesilva caendesilva merged commit f814d4f into refactor-hydefront-styles-to-tailwind Nov 16, 2024
8 checks passed
@caendesilva caendesilva deleted the recreate-the-hydesearch-plugin-with-alpine branch November 16, 2024 19:11
@caendesilva caendesilva mentioned this pull request Dec 10, 2024
99 tasks
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.

1 participant