From 1f074d34b5a364a33bf20a8f9f93cd2fc183ea43 Mon Sep 17 00:00:00 2001 From: Nathaniel Furniss Date: Sun, 18 Jul 2021 21:19:51 -0700 Subject: [PATCH 1/2] Remove LinkTo's tagName --- .../glimmer/lib/components/-link-to.ts | 30 ---------------- .../glimmer/lib/components/link-to.ts | 34 ------------------- 2 files changed, 64 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/-link-to.ts b/packages/@ember/-internals/glimmer/lib/components/-link-to.ts index f0a8e651e13..a633894c1f4 100644 --- a/packages/@ember/-internals/glimmer/lib/components/-link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/-link-to.ts @@ -201,27 +201,6 @@ import layout from '../templates/-link-to'; When transitioning into the linked route, the `model` hook will be triggered with parameters including this passed identifier. - ### Supplying a `tagName` - - By default `` renders an `` element. This can be overridden for a single use of - `` by supplying a `tagName` argument: - - ```handlebars - - Great Hamster Photos - - ``` - - This produces: - - ```html -
  • - Great Hamster Photos -
    • - ``` - - In general, this is not recommended. - ### Supplying query parameters If you need to add optional key-value pairs that appear to the right of the ? in a URL, @@ -791,9 +770,6 @@ const LinkComponent = EmberComponent.extend({ Sets the element's `href` attribute to the url for the `LinkComponent`'s targeted route. - If the `LinkComponent`'s `tagName` is changed to a value other - than `a`, this property will be ignored. - @property href @private */ @@ -802,14 +778,9 @@ const LinkComponent = EmberComponent.extend({ '_route', '_models', '_query', - 'tagName', 'loading', 'loadingHref', function computeLinkToComponentHref(this: any) { - if (this.tagName !== 'a') { - return; - } - if (this.loading) { return this.loadingHref; } @@ -869,7 +840,6 @@ const LinkComponent = EmberComponent.extend({ /** The default href value to use while a link-to is loading. - Only applies when tagName is 'a' @property loadingHref @type String diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index 3bb0b105281..810d1b303d1 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -418,40 +418,6 @@ if (EMBER_MODERNIZED_BUILT_IN_COMPONENTS) { }); } - // @tagName - { - let superOnUnsupportedArgument = prototype['onUnsupportedArgument']; - - Object.defineProperty(prototype, 'onUnsupportedArgument', { - configurable: true, - enumerable: false, - value: function onUnsupportedArgument(this: LinkTo, name: string): void { - if (name === 'tagName') { - let tagName = this.named('tagName'); - - deprecate( - `Passing the \`@tagName\` argument to is deprecated. Using a <${tagName}> ` + - 'element for navigation is not recommended as it creates issues with assistive ' + - 'technologies. Remove this argument to use the default     element. In the rare ' + - 'cases that calls for using a different element, refactor to use the router ' + - 'service inside a custom event handler instead.', - false, - { - id: 'ember.link-to.tag-name', - for: 'ember-source', - since: {}, - until: '4.0.0', - } - ); - - this.modernized = false; - } else { - superOnUnsupportedArgument.call(this, name); - } - }, - }); - } - // @bubbles & @preventDefault { let superIsSupportedArgument = prototype['isSupportedArgument']; From 827c35a6c4c2dab190c92c8d1dd2fa9bddbf2671 Mon Sep 17 00:00:00 2001 From: Nathaniel Furniss Date: Mon, 19 Jul 2021 23:35:52 -0700 Subject: [PATCH 2/2] maybe solution to tagName --- .../glimmer/lib/components/link-to.ts | 5 +++++ .../link-to/query-params-angle-test.js | 4 +--- .../link-to/rendering-angle-test.js | 6 ++++++ .../link-to/rendering-curly-test.js | 6 ++++++ .../components/link-to/routing-angle-test.js | 14 -------------- .../components/link-to/routing-curly-test.js | 19 ------------------- .../transitioning-classes-angle-test.js | 13 ++++--------- .../transitioning-classes-curly-test.js | 13 ++++--------- 8 files changed, 26 insertions(+), 54 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index 810d1b303d1..7469a6081e1 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -80,6 +80,11 @@ class LinkTo extends InternalComponent implements DeprecatingInternalComponent { !('model' in this.args.named && 'models' in this.args.named) ); + assert( + 'Passing the `@tagName` argument to is not supported.', + !('tagName' in this.args.named) + ); + super.validateArguments(); } diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-angle-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-angle-test.js index 2729071af47..e55ebb82d33 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-angle-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-angle-test.js @@ -351,12 +351,10 @@ moduleFor( } async ['@test supplied QP properties can be bound in legacy components'](assert) { - expectDeprecation(/Passing the `@tagName` argument to/); - this.addTemplate( 'index', ` - + Index ` diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-angle-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-angle-test.js index 3c555705a1c..87b6789e636 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-angle-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-angle-test.js @@ -171,6 +171,12 @@ moduleFor( content: 'Go to Index', }); } + + ['@test it should throw an error if `tagName` is passed in']() { + expectAssertion(() => { + this.render(`Go to Index`); + }, /Passing the `@tagName` argument to is not supported./); + } } ); diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-curly-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-curly-test.js index b934154d7ba..bd47b1163df 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-curly-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-curly-test.js @@ -192,5 +192,11 @@ moduleFor( this.assertText('Go to Index'); } + + ['@test it should throw an error if `tagName` is passed in']() { + expectAssertion(() => { + this.render(`{{#link-to route='index' tagName='button'}}Go to Index{{/link-to}}`); + }, /Passing the `@tagName` argument to is not supported./); + } } ); diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-angle-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-angle-test.js index f1d1cfc863c..e9c8ac43804 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-angle-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-angle-test.js @@ -133,20 +133,6 @@ moduleFor( ); } - async [`@test [DEPRECATED] it doesn't add an href when the tagName isn't 'a'`](assert) { - this.addTemplate( - 'index', - `About` - ); - - await expectDeprecationAsync( - () => this.visit('/'), - /Passing the `@tagName` argument to is deprecated\./, - EMBER_MODERNIZED_BUILT_IN_COMPONENTS - ); - assert.strictEqual(this.$('#about-link').attr('href'), null, 'there is no href attribute'); - } - async [`@test it applies a 'disabled' class when disabled`](assert) { this.addTemplate( 'index', diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-curly-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-curly-test.js index eb12ae9e343..6c199bf5534 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-curly-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/routing-curly-test.js @@ -133,25 +133,6 @@ moduleFor( ); } - async [`@test [DEPRECATED] it doesn't add an href when the tagName isn't 'a'`](assert) { - this.addTemplate( - 'index', - `` - ); - - await expectDeprecationAsync( - () => this.visit('/'), - /Passing the `@tagName` argument to is deprecated\./, - EMBER_MODERNIZED_BUILT_IN_COMPONENTS - ); - - assert.strictEqual( - this.$('#about-link > div').attr('href'), - null, - 'there is no href attribute' - ); - } - async [`@test it applies a 'disabled' class when disabled`](assert) { this.addTemplate( 'index', diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-angle-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-angle-test.js index 03395293b97..4bca32946d1 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-angle-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-angle-test.js @@ -1,6 +1,5 @@ import { RSVP } from '@ember/-internals/runtime'; import { Route } from '@ember/-internals/routing'; -import { EMBER_MODERNIZED_BUILT_IN_COMPONENTS } from '@ember/canary-features'; import { moduleFor, ApplicationTestCase, runTask } from 'internal-test-helpers'; function assertHasClass(assert, selector, label) { @@ -190,13 +189,13 @@ moduleFor( 'application', ` {{outlet}} - + Index - + About - + Other ` @@ -204,11 +203,7 @@ moduleFor( } async beforeEach() { - return expectDeprecationAsync( - () => this.visit('/'), - /Passing the `@tagName` argument to is deprecated\./, - EMBER_MODERNIZED_BUILT_IN_COMPONENTS - ); + return this.visit('/'); } resolveAbout() { diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-curly-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-curly-test.js index 8724c4ee335..e9026243a6d 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-curly-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/transitioning-classes-curly-test.js @@ -1,6 +1,5 @@ import { RSVP } from '@ember/-internals/runtime'; import { Route } from '@ember/-internals/routing'; -import { EMBER_MODERNIZED_BUILT_IN_COMPONENTS } from '@ember/canary-features'; import { moduleFor, ApplicationTestCase, runTask } from 'internal-test-helpers'; function assertHasClass(assert, selector, label) { @@ -190,13 +189,13 @@ moduleFor( 'application', ` {{outlet}} - {{#link-to route='index' tagName='li'}} + {{#link-to route='index'}} {{/link-to}} - {{#link-to route='parent-route.about' tagName='li'}} + {{#link-to route='parent-route.about'}} {{/link-to}} - {{#link-to route='parent-route.other' tagName='li'}} + {{#link-to route='parent-route.other'}} {{/link-to}} ` @@ -204,11 +203,7 @@ moduleFor( } async beforeEach() { - return expectDeprecationAsync( - () => this.visit('/'), - /Passing the `@tagName` argument to is deprecated\./, - EMBER_MODERNIZED_BUILT_IN_COMPONENTS - ); + return this.visit('/'); } resolveAbout() {