DEV: Deprecate mobile-specific templates (#29514)

We are moving away from the mobile-specific template pattern in favor of logical `{{#if}}` statements. This brings us closer to a standard Ember app, makes testing easier, and reduces duplicate code.

This commit includes some minor refactoring in the resolver & component-templates initializer, so that the mobile lookups happen on desktop, without actually being used. This allows us to print the deprecation message consistently, to improve visibility to developers.
This commit is contained in:
David Taylor 2024-11-01 14:51:12 +00:00 committed by GitHub
parent 397d41008e
commit 68c7c8e25d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 87 additions and 53 deletions

View File

@ -298,8 +298,17 @@ export function buildResolver(baseName) {
}
findMobileTemplate(parsedName) {
const result = this.findTemplate(parsedName, "mobile/");
if (result) {
deprecated(
`Mobile-specific hbs templates are deprecated. Use responsive CSS or {{#if this.site.mobileView}} instead. [${parsedName}]`,
{
id: "discourse.mobile-templates",
}
);
}
if (_options.mobileView) {
return this.findTemplate(parsedName, "mobile/");
return result;
}
}

View File

@ -1,6 +1,7 @@
import * as GlimmerManager from "@glimmer/manager";
import ClassicComponent from "@ember/component";
import { isTesting } from "discourse-common/config/environment";
import deprecated from "discourse-common/lib/deprecated";
import DiscourseTemplateMap from "discourse-common/lib/discourse-template-map";
const COLOCATED_TEMPLATE_OVERRIDES = new Map();
@ -37,8 +38,17 @@ export default {
let componentName = templateKey;
if (mobile) {
componentName = componentName.slice("mobile/".length);
deprecated(
`Mobile-specific hbs templates are deprecated. Use responsive CSS or {{#if this.site.mobileView}} instead. [${templateKey}]`,
{
id: "discourse.mobile-templates",
}
);
if (this.site.mobileView) {
componentName = componentName.slice("mobile/".length);
}
}
componentName = componentName.slice("components/".length);
const component = owner.resolveRegistration(`component:${componentName}`);
@ -79,16 +89,11 @@ export default {
const orderedOverrides = [
[pluginTemplates, "components/", false],
[themeTemplates, "components/", false],
[coreTemplates, "mobile/components/", true],
[pluginTemplates, "mobile/components/", true],
[themeTemplates, "mobile/components/", true],
];
if (this.site.mobileView) {
orderedOverrides.push(
[coreTemplates, "mobile/components/", true],
[pluginTemplates, "mobile/components/", true],
[themeTemplates, "mobile/components/", true]
);
}
for (const [map, prefix, mobile] of orderedOverrides) {
for (const [key, value] of map) {
if (key.startsWith(prefix)) {

View File

@ -7,8 +7,21 @@ import sinon from "sinon";
import { overrideThrowGjsError } from "discourse/instance-initializers/component-templates";
import { forceMobile } from "discourse/lib/mobile";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import { withSilencedDeprecationsAsync } from "discourse-common/lib/deprecated";
import { registerTemporaryModule } from "../helpers/temporary-module-helper";
function silenceMobileDeprecations(hooks) {
let unsilenceCallback;
hooks.beforeEach(() => {
const promise = new Promise((resolve) => (unsilenceCallback = resolve));
withSilencedDeprecationsAsync(
["discourse.mobile-templates"],
() => promise
);
});
hooks.afterEach(() => unsilenceCallback());
}
// eslint-disable-next-line ember/no-empty-glimmer-component-classes
class MockColocatedComponent extends Component {}
setComponentTemplate(hbs`Colocated Original`, MockColocatedComponent);
@ -98,7 +111,9 @@ function registerTemplateOnlyComponents() {
);
}
module("Integration | Initializers | plugin-component-templates", function () {
module("Integration | Initializers | plugin-component-templates", function (h) {
silenceMobileDeprecations(h);
module("template-only component definition behaviour", function (hooks) {
hooks.beforeEach(() => registerTemplateOnlyComponents());
setupRenderingTest(hooks);

View File

@ -1,6 +1,7 @@
import { setupTest } from "ember-qunit";
import { module, test } from "qunit";
import { registerTemporaryModule } from "discourse/tests/helpers/temporary-module-helper";
import { withSilencedDeprecations } from "discourse-common/lib/deprecated";
import DiscourseTemplateMap from "discourse-common/lib/discourse-template-map";
import { buildResolver, setResolverOption } from "discourse-common/resolver";
@ -247,29 +248,31 @@ module("Unit | Ember | resolver", function (hooks) {
setResolverOption("mobileView", true);
// Default with mobile/ added
lookupTemplate(
assert,
"template:foo",
"discourse/templates/mobile/foo",
"finding mobile version even if normal one is not present"
);
withSilencedDeprecations("discourse.mobile-templates", () => {
// Default with mobile/ added
lookupTemplate(
assert,
"template:foo",
"discourse/templates/mobile/foo",
"finding mobile version even if normal one is not present"
);
// Default with mobile preferred
lookupTemplate(
assert,
"template:bar",
"discourse/templates/mobile/bar",
"preferring mobile version when both mobile and normal versions are present"
);
// Default with mobile preferred
lookupTemplate(
assert,
"template:bar",
"discourse/templates/mobile/bar",
"preferring mobile version when both mobile and normal versions are present"
);
// Default when mobile not present
lookupTemplate(
assert,
"template:baz",
"discourse/templates/baz",
"falling back to a normal version when mobile version is not present"
);
// Default when mobile not present
lookupTemplate(
assert,
"template:baz",
"discourse/templates/baz",
"falling back to a normal version when mobile version is not present"
);
});
});
test("resolves templates to plugin and theme namespaces", function (assert) {
@ -327,29 +330,31 @@ module("Unit | Ember | resolver", function (hooks) {
setResolverOption("mobileView", true);
// Default with plugin template override
lookupTemplate(
assert,
"template:foo",
"discourse/plugins/my-plugin/discourse/templates/mobile/foo",
"finding plugin version even if normal one is not present"
);
withSilencedDeprecations("discourse.mobile-templates", () => {
// Default with plugin template override
lookupTemplate(
assert,
"template:foo",
"discourse/plugins/my-plugin/discourse/templates/mobile/foo",
"finding plugin version even if normal one is not present"
);
// Default with plugin mobile added, takes precedence over non-mobile
lookupTemplate(
assert,
"template:bar",
"discourse/plugins/my-plugin/discourse/templates/mobile/bar",
"preferring plugin mobile version when both non-mobile plugin version is also present"
);
// Default with plugin mobile added, takes precedence over non-mobile
lookupTemplate(
assert,
"template:bar",
"discourse/plugins/my-plugin/discourse/templates/mobile/bar",
"preferring plugin mobile version when both non-mobile plugin version is also present"
);
// Default with when non-plugin mobile version is present
lookupTemplate(
assert,
"template:baz",
"discourse/plugins/my-plugin/discourse/templates/mobile/baz",
"preferring plugin mobile version over non-plugin mobile version"
);
// Default with when non-plugin mobile version is present
lookupTemplate(
assert,
"template:baz",
"discourse/plugins/my-plugin/discourse/templates/mobile/baz",
"preferring plugin mobile version over non-plugin mobile version"
);
});
});
test("resolves templates with 'admin' prefix", function (assert) {