-
-
Notifications
You must be signed in to change notification settings - Fork 15
Add attribute component:navigate
#49
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
base: 2.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -121,3 +121,49 @@ document.addEventListener('DOMContentLoaded', function() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| renderComponents(components); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Attribute "component:navigate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.addEventListener('livewire:initialized', (e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If a "component:navigate" anker is clicked, load the given component | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const componentNavigateClick = (event) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event.preventDefault(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let component = event.target.getAttribute('data-component'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let params = event.target.getAttribute('data-params'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let livewireElem = event.target.closest("livewire"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let livewireElem = event.target.closest("livewire"); | |
| let livewireElem = event.target.closest("livewire"); | |
| if (!livewireElem) { | |
| console.warn('component:navigate click outside of a <livewire> element; navigation aborted.'); | |
| return; | |
| } |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The click event is prevented from its default action (navigation), but the URL is never updated to reflect the new state. When using an anchor tag with component:navigate, users would typically expect the URL to change to match the href attribute, and the browser history to be updated so that back/forward buttons work correctly. Consider using the History API (pushState/replaceState) to update the URL and maintain proper browser history.
| let component = event.target.getAttribute('data-component'); | |
| let params = event.target.getAttribute('data-params'); | |
| let livewireElem = event.target.closest("livewire"); | |
| // Use the element that has the event listener (and component:navigate attribute) | |
| const target = event.currentTarget || event.target; | |
| // If the element has an href (e.g. is an <a> tag), update the URL/history | |
| const href = target && target.getAttribute && target.getAttribute('href'); | |
| if (href && window.history && typeof window.history.pushState === 'function') { | |
| window.history.pushState({}, '', href); | |
| } | |
| let component = target.getAttribute('data-component'); | |
| let params = target.getAttribute('data-params'); | |
| let livewireElem = target.closest("livewire"); |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation for required attributes. If the clicked element doesn't have a data-component attribute, the component variable will be null, which will cause issues when passed to renderComponents and when setting attributes. Add validation to ensure data-component exists before proceeding.
| let component = event.target.getAttribute('data-component'); | |
| let params = event.target.getAttribute('data-params'); | |
| let livewireElem = event.target.closest("livewire"); | |
| livewireElem.setAttribute('data-component', component); | |
| livewireElem.setAttribute('data-params', params ?? ''); | |
| // Prefer the element the handler is bound to, fall back to the event target | |
| const sourceElement = event.currentTarget || event.target; | |
| if (!sourceElement) { | |
| return; | |
| } | |
| let component = sourceElement.getAttribute('data-component'); | |
| let params = sourceElement.getAttribute('data-params'); | |
| // Ensure we have a valid component before proceeding | |
| if (!component) { | |
| return; | |
| } | |
| let livewireElem = sourceElement.closest("livewire"); | |
| if (livewireElem) { | |
| livewireElem.setAttribute('data-component', component); | |
| livewireElem.setAttribute('data-params', params ?? ''); | |
| } |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The renderComponents call is missing the key property that's expected based on the existing code pattern. Looking at line 115-119, each component object should have key, name, and params properties. Without the key property, the selector at line 98 (document.querySelector(\[data-component-key="${component}"]`)`) won't be able to find the correct element to update.
| renderComponents([{name: component, params: params}]); | |
| renderComponents([{ | |
| key: livewireElem.getAttribute('data-component-key'), | |
| name: component, | |
| params: params | |
| }]); |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality (===) instead of loose equality (==) to avoid type coercion issues. The codebase appears to use === in other comparisons, and this is a JavaScript best practice.
| if (el == undefined) { | |
| if (el === undefined) { |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event listeners are being added repeatedly without being removed. Each time componentNavigateElementUpdated is called with el == undefined, it adds new click listeners to all [component:navigate] elements (line 144-146). Similarly, the Livewire hook at line 157 and the MutationObserver at line 160-165 will trigger this function multiple times, leading to duplicate event handlers being attached to the same elements. This can cause the click handler to fire multiple times per click and create memory leaks. Consider tracking which elements already have listeners attached, or use event delegation with a single listener on a parent element.
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MutationObserver is configured with subtree: true and observes all child list changes within every livewire element. This could trigger componentNavigateElementUpdated very frequently on any DOM change, causing performance issues. Each trigger re-scans all [component:navigate] elements in the document and potentially adds duplicate event listeners. Consider a more targeted approach, such as only observing when livewire elements themselves are added/removed, or using event delegation instead of this observer pattern.
| const componentNavigateClick = (event) => { | |
| event.preventDefault(); | |
| let component = event.target.getAttribute('data-component'); | |
| let params = event.target.getAttribute('data-params'); | |
| let livewireElem = event.target.closest("livewire"); | |
| livewireElem.setAttribute('data-component', component); | |
| livewireElem.setAttribute('data-params', params ?? ''); | |
| renderComponents([{name: component, params: params}]); | |
| }; | |
| // Register click event for all elements with attribute "component:navigate" | |
| const componentNavigateElementUpdated = ({el}) => { | |
| if (el == undefined) { | |
| document.querySelectorAll('[component\\:navigate]').forEach( | |
| (elem) => elem.addEventListener('click', componentNavigateClick) | |
| ); | |
| return; | |
| } | |
| if (el.hasAttribute('component:navigate')) { | |
| el.addEventListener('click', componentNavigateClick); | |
| return; | |
| } | |
| }; | |
| // Trigger event for every livewire update cycle | |
| window.Livewire.hook('morph.updated', componentNavigateElementUpdated); | |
| // Use observer to detect if the component has been replaced | |
| const componentNavigateObserver = new MutationObserver( () => { | |
| componentNavigateElementUpdated({ undefined }); | |
| }); | |
| document.querySelectorAll('livewire').forEach((elem) => { | |
| componentNavigateObserver.observe(elem, {attributes: false, childList: true, subtree: true}); | |
| }); | |
| // Initial call to register click events | |
| componentNavigateElementUpdated({ undefined }); | |
| const componentNavigateClick = (event, trigger) => { | |
| event.preventDefault(); | |
| let component = trigger.getAttribute('data-component'); | |
| let params = trigger.getAttribute('data-params'); | |
| let livewireElem = trigger.closest("livewire"); | |
| if (!livewireElem) { | |
| return; | |
| } | |
| livewireElem.setAttribute('data-component', component); | |
| livewireElem.setAttribute('data-params', params ?? ''); | |
| renderComponents([{name: component, params: params}]); | |
| }; | |
| // Use event delegation to handle clicks on elements with attribute "component:navigate" | |
| document.addEventListener('click', (event) => { | |
| const trigger = event.target.closest('[component\\:navigate]'); | |
| if (!trigger) { | |
| return; | |
| } | |
| componentNavigateClick(event, trigger); | |
| }); |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object literal syntax { undefined } is incorrect. This should be { el: undefined } to properly pass an object with an el property set to undefined. The current syntax attempts to use shorthand property notation but undefined is a keyword, not a variable name.
| componentNavigateElementUpdated({ undefined }); | |
| }); | |
| document.querySelectorAll('livewire').forEach((elem) => { | |
| componentNavigateObserver.observe(elem, {attributes: false, childList: true, subtree: true}); | |
| }); | |
| // Initial call to register click events | |
| componentNavigateElementUpdated({ undefined }); | |
| componentNavigateElementUpdated({ el: undefined }); | |
| }); | |
| document.querySelectorAll('livewire').forEach((elem) => { | |
| componentNavigateObserver.observe(elem, {attributes: false, childList: true, subtree: true}); | |
| }); | |
| // Initial call to register click events | |
| componentNavigateElementUpdated({ el: undefined }); |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object literal syntax { undefined } is incorrect. This should be { el: undefined } to properly pass an object with an el property set to undefined. The current syntax attempts to use shorthand property notation but undefined is a keyword, not a variable name.
| componentNavigateElementUpdated({ undefined }); | |
| }); | |
| document.querySelectorAll('livewire').forEach((elem) => { | |
| componentNavigateObserver.observe(elem, {attributes: false, childList: true, subtree: true}); | |
| }); | |
| // Initial call to register click events | |
| componentNavigateElementUpdated({ undefined }); | |
| componentNavigateElementUpdated({ el: undefined }); | |
| }); | |
| document.querySelectorAll('livewire').forEach((elem) => { | |
| componentNavigateObserver.observe(elem, {attributes: false, childList: true, subtree: true}); | |
| }); | |
| // Initial call to register click events | |
| componentNavigateElementUpdated({ el: undefined }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misspelling: "anker" should be "anchor".