From ab4f2791c1968adbb1d858c5f009fa251bb9101b Mon Sep 17 00:00:00 2001 From: Brandy Carney Date: Mon, 5 Aug 2024 17:35:26 -0400 Subject: [PATCH] fix(vue): pass router-link value to href to properly render clickable elements (#29745) Issue number: N/A --------- ## What is the current behavior? Ionic Framework Vue components using `router-link` do not apply an `href` property which causes components to render `div` or `button` elements when they should render an `a`. This is inconsistent with the way Angular and Vue handle router link. ## What is the new behavior? Updates `@stencil/vue-output-target` to latest which adds the code from the following PR: https://github.com/ionic-team/stencil-ds-output-targets/pull/446 The update in vue output target checks if `router-link` and `navManager` are defined so this fix only applies to Ionic Framework components. If both are defined then it adds the `href` property to the element with the value of `router-link`. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `8.2.7-dev.11722629362.1ac136c4` --- core/package-lock.json | 14 ++++---- core/package.json | 2 +- packages/vue/scripts/sync.sh | 3 ++ packages/vue/src/vue-component-lib/utils.ts | 34 +++++++++++++++++-- .../vue/test/base/src/views/Components.vue | 6 ++-- packages/vue/test/base/src/views/Home.vue | 26 +++++++------- packages/vue/test/base/src/views/Routing.vue | 14 ++++---- packages/vue/test/base/src/views/Tab1.vue | 8 ++--- packages/vue/test/base/src/views/Tab2.vue | 2 +- .../test/base/tests/e2e/specs/routing.cy.js | 6 ++++ 10 files changed, 77 insertions(+), 38 deletions(-) diff --git a/core/package-lock.json b/core/package-lock.json index ca36bbe236f..cb09bac3018 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -28,7 +28,7 @@ "@stencil/angular-output-target": "^0.8.4", "@stencil/react-output-target": "^0.5.3", "@stencil/sass": "^3.0.9", - "@stencil/vue-output-target": "^0.8.7", + "@stencil/vue-output-target": "^0.8.9", "@types/jest": "^29.5.6", "@types/node": "^14.6.0", "@typescript-eslint/eslint-plugin": "^6.7.2", @@ -1858,9 +1858,9 @@ } }, "node_modules/@stencil/vue-output-target": { - "version": "0.8.8", - "resolved": "https://registry.npmjs.org/@stencil/vue-output-target/-/vue-output-target-0.8.8.tgz", - "integrity": "sha512-xrpz92lmTuLgwY9CLgl2Q14+zBXfBuXiRS6uXFPfeaFo0pe+do8cZitOOQ8i8tcoCa/tAqgD9B9CD+yQehSIGg==", + "version": "0.8.9", + "resolved": "https://registry.npmjs.org/@stencil/vue-output-target/-/vue-output-target-0.8.9.tgz", + "integrity": "sha512-1yuapCWYViLlxGlEaeta2wryq4M5zZxxBa+4rEBp54VwW2W/trlzPv0IJyw6I3Il51rHYm2WmWlBLOGmoMyW9Q==", "dev": true, "peerDependencies": { "@stencil/core": ">=2.0.0 || >=3 || >= 4.0.0-beta.0 || >= 4.0.0" @@ -11602,9 +11602,9 @@ "requires": {} }, "@stencil/vue-output-target": { - "version": "0.8.8", - "resolved": "https://registry.npmjs.org/@stencil/vue-output-target/-/vue-output-target-0.8.8.tgz", - "integrity": "sha512-xrpz92lmTuLgwY9CLgl2Q14+zBXfBuXiRS6uXFPfeaFo0pe+do8cZitOOQ8i8tcoCa/tAqgD9B9CD+yQehSIGg==", + "version": "0.8.9", + "resolved": "https://registry.npmjs.org/@stencil/vue-output-target/-/vue-output-target-0.8.9.tgz", + "integrity": "sha512-1yuapCWYViLlxGlEaeta2wryq4M5zZxxBa+4rEBp54VwW2W/trlzPv0IJyw6I3Il51rHYm2WmWlBLOGmoMyW9Q==", "dev": true, "requires": {} }, diff --git a/core/package.json b/core/package.json index 72fb83e7bd1..0d7a00323f6 100644 --- a/core/package.json +++ b/core/package.json @@ -50,7 +50,7 @@ "@stencil/angular-output-target": "^0.8.4", "@stencil/react-output-target": "^0.5.3", "@stencil/sass": "^3.0.9", - "@stencil/vue-output-target": "^0.8.7", + "@stencil/vue-output-target": "^0.8.9", "@types/jest": "^29.5.6", "@types/node": "^14.6.0", "@typescript-eslint/eslint-plugin": "^6.7.2", diff --git a/packages/vue/scripts/sync.sh b/packages/vue/scripts/sync.sh index 1bc4944ded7..4d26d15f2ba 100644 --- a/packages/vue/scripts/sync.sh +++ b/packages/vue/scripts/sync.sh @@ -5,6 +5,9 @@ set -e # Delete old packages rm -f *.tgz +# Delete vite cache +rm -rf node_modules/.vite + # Pack @ionic/core npm pack ../../core diff --git a/packages/vue/src/vue-component-lib/utils.ts b/packages/vue/src/vue-component-lib/utils.ts index 136041073e8..0d99b6696d7 100644 --- a/packages/vue/src/vue-component-lib/utils.ts +++ b/packages/vue/src/vue-component-lib/utils.ts @@ -91,8 +91,17 @@ export const defineContainer = ( const eventsNames = Array.isArray(modelUpdateEvent) ? modelUpdateEvent : [modelUpdateEvent]; eventsNames.forEach((eventName: string) => { el.addEventListener(eventName.toLowerCase(), (e: Event) => { - modelPropValue = (e?.target as any)[modelProp]; - emit(UPDATE_VALUE_EVENT, modelPropValue); + /** + * Only update the v-model binding if the event's target is the element we are + * listening on. For example, Component A could emit ionChange, but it could also + * have a descendant Component B that also emits ionChange. We only want to update + * the v-model for Component A when ionChange originates from that element and not + * when ionChange bubbles up from Component B. + */ + if (e.target.tagName === el.tagName) { + modelPropValue = (e?.target as any)[modelProp]; + emit(UPDATE_VALUE_EVENT, modelPropValue); + } }); }); }, @@ -106,6 +115,16 @@ export const defineContainer = ( if (routerLink === EMPTY_PROP) return; if (navManager !== undefined) { + /** + * This prevents the browser from + * performing a page reload when pressing + * an Ionic component with routerLink. + * The page reload interferes with routing + * and causes ion-back-button to disappear + * since the local history is wiped on reload. + */ + ev.preventDefault(); + let navigationPayload: any = { event: ev }; for (const key in props) { const value = props[key]; @@ -176,6 +195,17 @@ export const defineContainer = ( } } + // If router link is defined, add href to props + // in order to properly render an anchor tag inside + // of components that should become activatable and + // focusable with router link. + if (props[ROUTER_LINK_VALUE] !== EMPTY_PROP) { + propsToAdd = { + ...propsToAdd, + href: props[ROUTER_LINK_VALUE], + }; + } + /** * vModelDirective is only needed on components that support v-model. * As a result, we conditionally call withDirectives with v-model components. diff --git a/packages/vue/test/base/src/views/Components.vue b/packages/vue/test/base/src/views/Components.vue index cad54df3631..01b11821ad2 100644 --- a/packages/vue/test/base/src/views/Components.vue +++ b/packages/vue/test/base/src/views/Components.vue @@ -2,13 +2,13 @@ - + Breadcrumbs - + Select - + Range diff --git a/packages/vue/test/base/src/views/Home.vue b/packages/vue/test/base/src/views/Home.vue index a0652a93b03..0f04f228904 100644 --- a/packages/vue/test/base/src/views/Home.vue +++ b/packages/vue/test/base/src/views/Home.vue @@ -17,43 +17,43 @@ - + Overlays - + Icons - + Inputs - + Navigation - + Routing - + Default Href - + Nested Router Outlet - + Tabs - + Tabs Secondary - + Lifecycle - + Lifecycle (Setup) - + Delayed Redirect - + Components diff --git a/packages/vue/test/base/src/views/Routing.vue b/packages/vue/test/base/src/views/Routing.vue index f4f3c671643..f7977dde788 100644 --- a/packages/vue/test/base/src/views/Routing.vue +++ b/packages/vue/test/base/src/views/Routing.vue @@ -16,31 +16,31 @@ - + Set Route Parameters - + Go to Child Page - + Go to Parameter Page ABC - + Go to Parameter Page XYZ - + Go to Parameterized Page View - + Replace to Navigation page - + Go to /tabs/tab1 diff --git a/packages/vue/test/base/src/views/Tab1.vue b/packages/vue/test/base/src/views/Tab1.vue index e9ad7eb19f4..40e15a4ad44 100644 --- a/packages/vue/test/base/src/views/Tab1.vue +++ b/packages/vue/test/base/src/views/Tab1.vue @@ -17,18 +17,18 @@ - + Go to Tab 1 Child 1 - + Go to Nested Outlet - + Go to Secondary Tabs - + Go to Primary Tabs diff --git a/packages/vue/test/base/src/views/Tab2.vue b/packages/vue/test/base/src/views/Tab2.vue index 755e8e26ba3..25bfbcac4df 100644 --- a/packages/vue/test/base/src/views/Tab2.vue +++ b/packages/vue/test/base/src/views/Tab2.vue @@ -15,7 +15,7 @@ - + Go to /routing diff --git a/packages/vue/test/base/tests/e2e/specs/routing.cy.js b/packages/vue/test/base/tests/e2e/specs/routing.cy.js index d3498bca7d8..289df4c55a9 100644 --- a/packages/vue/test/base/tests/e2e/specs/routing.cy.js +++ b/packages/vue/test/base/tests/e2e/specs/routing.cy.js @@ -584,4 +584,10 @@ describe('Routing - Swipe to Go Back', () => { cy.ionPageDoesNotExist('routingparameter-abc'); cy.ionPageVisible('routing'); }) + + it('should be activatable when router-link is used on an item without the button property', () => { + cy.visit('/'); + + cy.get('ion-item[routerlink="/overlays"]').should('have.class', 'ion-activatable'); + }); })