Skip to content

Commit

Permalink
Loading options as expressions
Browse files Browse the repository at this point in the history
- the option `loading-offset` is now deprecated
- the option `loading` is now processed as expression, same as the `fetchpriority` option
- options from the AMP application have higher priority than options from the client definitions
- added method `Options.evaluate()`
  • Loading branch information
tg666 committed Aug 29, 2024
1 parent 040996b commit 9d89e41
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/banner/banner.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ export class Banner {
return null;
}

mergeOptions(options) {
this.#options.merge(options);
overrideOptions(options) {
this.#options.override(options);
}

redrawIfNeeded() {
Expand Down
2 changes: 1 addition & 1 deletion src/banner/managed/managed-banner.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export class ManagedBanner extends Banner {
});

if ('options' in responseData) {
this.mergeOptions(responseData.options);
this.overrideOptions(responseData.options);
}

const banners = [];
Expand Down
43 changes: 40 additions & 3 deletions src/banner/options.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { evaluateExpression } from '../utils/expression.mjs';

export class Options {
constructor(options) {
this.options = options;
this.options = {};
this.options = this.#fixOptionsCompatibility({...options});
}

has(optionName) {
Expand All @@ -11,7 +14,41 @@ export class Options {
return this.options[optionName] || defaultValue;
}

merge(options) {
this.options = {...this.options, ...options};
evaluate(optionName, index) {
if (undefined === this.options[optionName]) {
return null;
}

return evaluateExpression(
this.options[optionName],
index
);
}

override(options) {
this.options = {
...this.options,
...this.#fixOptionsCompatibility({...options}),
};
}

#fixOptionsCompatibility(options) {
if ('loading-offset' in options) {
const offset = options['loading-offset'];
const loading = 'loading' in options ? options['loading'] : ('loading' in this.options ? this.options['loading'] : null);
const loadingExpression = null !== loading ? `>=${offset}:${loading}` : null;
let message = `AMP deprecation warning: The banner option "loading-offset" is deprecated and will be removed in some future release.`;

if (null !== loadingExpression) {
message += ` Instead of options {"loading": "${loading}", "loading-offset": "${offset}"} use an expression based option {"loading": "${loadingExpression}"}.`;
}

console.warn(message);

delete options['loading-offset'];
null !== loadingExpression && (options.loading = loadingExpression);
}

return options;
}
}
2 changes: 1 addition & 1 deletion src/client/standard/client.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class Client {

if ('embed' === positionData.mode) {
if ('options' in positionData) {
banner.mergeOptions(positionData.options);
banner.overrideOptions(positionData.options);
}

this.createBanner(banner.element, banner.position, banner.rawResources, banner.options.options, positionData.mode);
Expand Down
2 changes: 0 additions & 2 deletions src/renderer/banner-renderer.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { TemplateLoader } from './temnplate-loader.mjs';
import { evaluateExpression } from '../utils/expression.mjs';

export class BannerRenderer {
#loader;
Expand All @@ -15,7 +14,6 @@ export class BannerRenderer {
return this.#loader.getTemplate(banner.positionData.displayType)({
banner: banner,
data: banner.bannerData,
expr: evaluateExpression,
});
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/template/multiple.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* {ManagedBanner} banner
* {Array<BannerData>} data
* {Function(String expression, Integer index): String|null} expr
*/
export default `
<div class="amp-banner amp-banner--multiple">
Expand All @@ -27,8 +26,8 @@ export default `
<% if(null !== banner.positionData.dimensions.width) { %>width="<%- banner.positionData.dimensions.width %>"<% } %>
<% if(null !== banner.positionData.dimensions.height) { %>height="<%- banner.positionData.dimensions.height %>"<% } %>
<% if('' !== b.content.title) { %>title="<%- b.content.title %>"<% } %>
<% if(banner.options.has('loading') && index >= parseInt(banner.options.get('loading-offset', 0))) { %>loading="<%- banner.options.get('loading') %>"<% } %>
<% if(banner.options.has('fetchpriority') && expr(banner.options.get('fetchpriority'), index)) { %>fetchpriority="<%- expr(banner.options.get('fetchpriority'), index) %>"<% } %>>
<% var loading; if(loading = banner.options.evaluate('loading', index)) { %>loading="<%- loading %>"<% } %>
<% var fetchPriority; if(fetchPriority = banner.options.evaluate('fetchpriority', index)) { %>fetchpriority="<%- fetchPriority %>"<% } %>>
</picture>
</a>
<% } else if ('html' === b.content.type) { %>
Expand Down
5 changes: 2 additions & 3 deletions src/template/random.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* {ManagedBanner} banner
* {BannerData} data
* {Function(String expression, Integer index): String|null} expr
*/
export default `
<div class="amp-banner amp-banner--random"
Expand All @@ -24,8 +23,8 @@ export default `
<% if(null !== banner.positionData.dimensions.width) { %>width="<%- banner.positionData.dimensions.width %>"<% } %>
<% if(null !== banner.positionData.dimensions.height) { %>height="<%- banner.positionData.dimensions.height %>"<% } %>
<% if('' !== data.content.title) { %>title="<%- data.content.title %>"<% } %>
<% if(banner.options.has('loading')) { %>loading="<%- banner.options.get('loading') %>"<% } %>
<% if(banner.options.has('fetchpriority') && expr(banner.options.get('fetchpriority'), 0)) { %>fetchpriority="<%- expr(banner.options.get('fetchpriority'), 0) %>"<% } %>>
<% var loading; if(loading = banner.options.evaluate('loading', 0)) { %>loading="<%- loading %>"<% } %>
<% var fetchPriority; if(fetchPriority = banner.options.evaluate('fetchpriority', 0)) { %>fetchpriority="<%- fetchPriority %>"<% } %>>
</picture>
</a>
<% } else if ('html' === data.content.type) { %>
Expand Down
5 changes: 2 additions & 3 deletions src/template/single.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* {ManagedBanner} banner
* {BannerData} data
* {Function(String expression, Integer index): String|null} expr
*/
export default `
<div class="amp-banner amp-banner--single"
Expand All @@ -24,8 +23,8 @@ export default `
<% if(null !== banner.positionData.dimensions.width) { %>width="<%- banner.positionData.dimensions.width %>"<% } %>
<% if(null !== banner.positionData.dimensions.height) { %>height="<%- banner.positionData.dimensions.height %>"<% } %>
<% if('' !== data.content.title) { %>title="<%- data.content.title %>"<% } %>
<% if(banner.options.has('loading')) { %>loading="<%- banner.options.get('loading') %>"<% } %>
<% if(banner.options.has('fetchpriority') && expr(banner.options.get('fetchpriority'), 0)) { %>fetchpriority="<%- expr(banner.options.get('fetchpriority'), 0) %>"<% } %>>
<% var loading; if(loading = banner.options.evaluate('loading', 0)) { %>loading="<%- loading %>"<% } %>
<% var fetchPriority; if(fetchPriority = banner.options.evaluate('fetchpriority', 0)) { %>fetchpriority="<%- fetchPriority %>"<% } %>>
</picture>
</a>
<% } else if ('html' === data.content.type) { %>
Expand Down

0 comments on commit 9d89e41

Please sign in to comment.