Skip to content

Commit

Permalink
fix(vue): pass router-link value to href to properly render clickable…
Browse files Browse the repository at this point in the history
… 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:
ionic-team/stencil-ds-output-targets#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`
  • Loading branch information
brandyscarney committed Aug 5, 2024
1 parent a9f278a commit ab4f279
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 38 deletions.
14 changes: 7 additions & 7 deletions core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions packages/vue/scripts/sync.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
34 changes: 32 additions & 2 deletions packages/vue/src/vue-component-lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,17 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
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);
}
});
});
},
Expand All @@ -106,6 +115,16 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
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];
Expand Down Expand Up @@ -176,6 +195,17 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
}
}

// 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.
Expand Down
6 changes: 3 additions & 3 deletions packages/vue/test/base/src/views/Components.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
<ion-page>
<ion-content>
<ion-list>
<ion-item button router-link="/components/breadcrumbs">
<ion-item router-link="/components/breadcrumbs">
<ion-label>Breadcrumbs</ion-label>
</ion-item>
<ion-item button router-link="/components/select">
<ion-item router-link="/components/select">
<ion-label>Select</ion-label>
</ion-item>
<ion-item button router-link="/components/range">
<ion-item router-link="/components/range">
<ion-label>Range</ion-label>
</ion-item>
</ion-list>
Expand Down
26 changes: 13 additions & 13 deletions packages/vue/test/base/src/views/Home.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,43 +17,43 @@
</ion-header>

<ion-list>
<ion-item button router-link="/overlays">
<ion-item router-link="/overlays">
<ion-label>Overlays</ion-label>
</ion-item>
<ion-item button router-link="/icons">
<ion-item router-link="/icons">
<ion-label>Icons</ion-label>
</ion-item>
<ion-item button router-link="/inputs">
<ion-item router-link="/inputs">
<ion-label>Inputs</ion-label>
</ion-item>
<ion-item button router-link="/navigation" id="navigation">
<ion-item router-link="/navigation" id="navigation">
<ion-label>Navigation</ion-label>
</ion-item>
<ion-item button router-link="/routing" id="routing">
<ion-item router-link="/routing" id="routing">
<ion-label>Routing</ion-label>
</ion-item>
<ion-item button router-link="/default-href" id="default-href">
<ion-item router-link="/default-href" id="default-href">
<ion-label>Default Href</ion-label>
</ion-item>
<ion-item button router-link="/nested" id="nested">
<ion-item router-link="/nested" id="nested">
<ion-label>Nested Router Outlet</ion-label>
</ion-item>
<ion-item button router-link="/tabs" id="tabs">
<ion-item router-link="/tabs" id="tabs">
<ion-label>Tabs</ion-label>
</ion-item>
<ion-item button router-link="/tabs-secondary" id="tab-secondary">
<ion-item router-link="/tabs-secondary" id="tab-secondary">
<ion-label>Tabs Secondary</ion-label>
</ion-item>
<ion-item button router-link="/lifecycle" id="lifecycle">
<ion-item router-link="/lifecycle" id="lifecycle">
<ion-label>Lifecycle</ion-label>
</ion-item>
<ion-item button router-link="/lifecycle-setup" id="lifecycle-setup">
<ion-item router-link="/lifecycle-setup" id="lifecycle-setup">
<ion-label>Lifecycle (Setup)</ion-label>
</ion-item>
<ion-item button router-link="/delayed-redirect" id="delayed-redirect">
<ion-item router-link="/delayed-redirect" id="delayed-redirect">
<ion-label>Delayed Redirect</ion-label>
</ion-item>
<ion-item button router-link="/components">
<ion-item router-link="/components">
<ion-label>Components</ion-label>
</ion-item>
</ion-list>
Expand Down
14 changes: 7 additions & 7 deletions packages/vue/test/base/src/views/Routing.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,31 @@
</ion-toolbar>
</ion-header>

<ion-item button @click="setRouteParams" id="route-params">
<ion-item @click="setRouteParams" id="route-params">
<ion-label>Set Route Parameters</ion-label>
</ion-item>

<ion-item button router-link="/routing/child" id="child">
<ion-item router-link="/routing/child" id="child">
<ion-label>Go to Child Page</ion-label>
</ion-item>

<ion-item button router-link="/routing/abc" id="parameter-abc">
<ion-item router-link="/routing/abc" id="parameter-abc">
<ion-label>Go to Parameter Page ABC</ion-label>
</ion-item>

<ion-item button router-link="/routing/xyz" id="parameter-xyz">
<ion-item router-link="/routing/xyz" id="parameter-xyz">
<ion-label>Go to Parameter Page XYZ</ion-label>
</ion-item>

<ion-item button router-link="/routing/123/view" id="parameter-view-item">
<ion-item router-link="/routing/123/view" id="parameter-view-item">
<ion-label>Go to Parameterized Page View</ion-label>
</ion-item>

<ion-item button @click="replace" id="replace">
<ion-item @click="replace" id="replace">
<ion-label>Replace to Navigation page</ion-label>
</ion-item>

<ion-item button router-link="/tabs/tab1" id="tab1">
<ion-item router-link="/tabs/tab1" id="tab1">
<ion-label>Go to /tabs/tab1</ion-label>
</ion-item>
</ion-content>
Expand Down
8 changes: 4 additions & 4 deletions packages/vue/test/base/src/views/Tab1.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@

<ExploreContainer name="Tab 1 page" />

<ion-item button router-link="/tabs/tab1/childone" id="child-one">
<ion-item router-link="/tabs/tab1/childone" id="child-one">
<ion-label>Go to Tab 1 Child 1</ion-label>
</ion-item>
<ion-item button router-link="/nested" id="nested">
<ion-item router-link="/nested" id="nested">
<ion-label>Go to Nested Outlet</ion-label>
</ion-item>

<ion-item button router-link="/tabs-secondary" id="tabs-secondary">
<ion-item router-link="/tabs-secondary" id="tabs-secondary">
<ion-label>Go to Secondary Tabs</ion-label>
</ion-item>

<ion-item button router-link="/tabs" id="tabs-primary">
<ion-item router-link="/tabs" id="tabs-primary">
<ion-label>Go to Primary Tabs</ion-label>
</ion-item>

Expand Down
2 changes: 1 addition & 1 deletion packages/vue/test/base/src/views/Tab2.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</ion-toolbar>
</ion-header>

<ion-item button router-link="/routing" id="routing">
<ion-item router-link="/routing" id="routing">
<ion-label>Go to /routing</ion-label>
</ion-item>
</ion-content>
Expand Down
6 changes: 6 additions & 0 deletions packages/vue/test/base/tests/e2e/specs/routing.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
})

0 comments on commit ab4f279

Please sign in to comment.