feat: add wp-scaffold:elasticsearch#17
Conversation
|
Coverage report for commit: e1a3699 Summary - Lines: 79.71% | Methods: 22.22%
🤖 comment via lucassabreu/comment-coverage-clover |
||||||||||||||||||||||||||||||||||||||||||||
0d38271 to
3d641e4
Compare
| 'web/app/themes/' . get_stylesheet() . '/resources/scripts/reactive-search/reactive-search.jsx', | ||
| ])->toHtml(); | ||
| } catch (\Exception $e) { | ||
| // Fail silently if entrypoint is not found. |
There was a problem hiding this comment.
Misschien hier nog een visuele error tonen? Want vind @yardinternet/wordpress-backend ?
| '--provider' => 'Yard\\Brave\\Scaffold\\ScaffoldServiceProvider', | ||
| '--tag' => 'elasticsearch', | ||
| ]); | ||
| $this->info('Please follow the instructions in the documentation to complete the Elasticsearch setup:'); |
There was a problem hiding this comment.
You need to do some additional steps after running this scaffold. Please read the docs here:
(meer forceful :P )
ik lul een beetje maar doe wat je wilt, beetje verschil tussen "Please" en "You need to"
There was a problem hiding this comment.
Veel beter, veranderd
| #[Action('template_redirect')] | ||
| public function redirectDefaultSearch(): void | ||
| { | ||
| if (! is_search() || is_admin() || ! empty($_GET['q'])) { |
There was a problem hiding this comment.
if (! is_search() || is_admin() || (isset($_GET['q']) && '' !== trim($_GET['q']))){
Maar onderstaande is leesbaarder:
$hasQueryParam = isset($_GET['q']) && '' !== trim($_GET['q']);
if (! is_search() || is_admin() || $hasQueryParam ){
There was a problem hiding this comment.
Ik kwam er n.a.v. Lara achter dat een aantal van deze functies al in Brave Hooks staat: https://github.com/yardinternet/brave-hooks/blob/main/src/Elasticsearch.php
Heb ze hier weggehaald en alleen het enqueue'en van Reactive Search toegevoegd + reactive-search-page.blade.php template include hier gelaten.
| return; | ||
| } | ||
|
|
||
| $searchQuery = sanitize_text_field(get_query_var('s')); |
There was a problem hiding this comment.
Wil je niet early returnen als get_query_var('s') een lege string is?
| use Yard\Hook\Filter; | ||
|
|
||
| #[Plugin('yard-elasticsearch/yard-elasticsearch.php')] | ||
| class Elasticsearch |
There was a problem hiding this comment.
Volgens mij staan heel veel van deze hooks in brave hooks, bij Involv ziet het bestand er zo uit: https://github.com/yardinternet/involv/blob/main/web/app/themes/sage/app/Hooks/Elasticsearch.php
There was a problem hiding this comment.
Thanks! Dit stond TOTAAL niet op mijn radar. Heb de functies verwijderd!
| primaryColor: '#3a0056', | ||
| borderColor: '#3a0056', |
There was a problem hiding this comment.
Moeten we hier eigenlijk niet een soort TODO bijschrijven, ik merk namelijk dat deze vaak vergeten wordt om aan te passen. Als iemand dan een keer op TODO in het hele project zoekt, dan komt ie het in ieder geval tegen. Niet ideaal maar ik zou ook niks anders weten.
There was a problem hiding this comment.
Ja goeie. @todo wordt doorgaans gebruikt in docblocks (JS en PHP in ieder geval, zie bijv. https://jsdoc.app/tags-todo). We kunnen dit als conventie gebruiken!
| <ReactiveSearchBar | ||
| boosts={ window.YS.boosts } | ||
| dataFields={ window.YS.dataFields } | ||
| fuzziness={ window.YS.fuzziness } |
There was a problem hiding this comment.
Volgens mij geeft dit nog soms warnings dat het geen int is. Ik heb dit in Involv staan (wat ik ook weer ergens vandaan heb gehaald):
fuzziness={
window.YS.fuzziness === 'AUTO'
? window.YS.fuzziness
: parseInt( window.YS.fuzziness )
}
There was a problem hiding this comment.
Goeie, toegevoegd
| @apply shadow-none; | ||
|
|
||
| .yrs-datasearch-autosuggest-input { | ||
| @apply ring-primary focus:border-primary h-(--yrs-search-input-height) rounded-none !py-3 !pr-16 !pl-4 shadow-none; |
There was a problem hiding this comment.
We moeten als frontenders ook gaan bedenken wat we met de ! gaan doen, gaan we het ervoor of erna zetten als nieuwe syntax. Als we de linting blij moeten maken, dan moeten we het erna gaan zetten. Als we het erna gaan doen, dan is het handig als we het gelijk hier ook goed zetten.
There was a problem hiding this comment.
Ik heb het er met een paar mensen over gehad, ! erna heeft wel de voorkeur. Linter happy, én het is volgens de conventie dat !important achter de property value komt. Heb het ook aangepast hier. Wat vind jij?
@WybeBosch heeft een slimme regex gemaakt om dit in een project in één keer door te voeren. Dit moet hij nog in Brave doen!
| <ReactiveSearchInput | ||
| boosts={ window.YS.boosts } | ||
| dataFields={ window.YS.dataFields } | ||
| fuzziness={ window.YS.fuzziness } |
There was a problem hiding this comment.
Hier ook dezelfde opmerking
| boosts={ window.YS.boosts } | ||
| dataField={ window.YS.dataFields } | ||
| filters={ window.YS.filters } | ||
| fuzziness={ window.YS.fuzziness } |
f659039 to
4ddc2dd
Compare
4ddc2dd to
e1a3699
Compare
Makkelijk gemaakt om Elasticsearch + Reactive Search te scaffolden 🎉
Na de scaffold moet je de docs/elasticsearch.md nog volgen om het volledig werkend te maken. Je krijgt na de scaffold een mooi linkje naar de docs: