Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(imsize): rework image styling to get more control #549

Merged
merged 5 commits into from
Nov 7, 2024
Merged

feat(imsize): rework image styling to get more control #549

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bbf59b6
feat(imsize): rework image styling to get more control
St1ggy Nov 1, 2024
c5296a1
chore: add more tests with different size configs, refactor styling b…
St1ggy Nov 2, 2024
879e32d
chore: hide inline size styling behind feature flag
St1ggy Nov 6, 2024
648f539
chore: add forcedSanitizeWhiteList
St1ggy Nov 6, 2024
d6ce8fb
chore: rename params
St1ggy Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/transform/md.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ function initCompiler(md: MarkdownIt, options: OptionsType, env: EnvType) {
const html = md.renderer.render(tokens, md.options, env);

// Sanitize the page
return needToSanitizeHtml ? sanitizeHtml(html, sanitizeOptions) : html;
return needToSanitizeHtml
? sanitizeHtml(html, sanitizeOptions, env.forcedSanitizeCssWhiteList)
: html;
};
}

Expand Down
1 change: 1 addition & 0 deletions src/transform/plugins/imsize/const.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export enum ImsizeAttr {
Title = 'title',
Width = 'width',
Height = 'height',
Style = 'style',
}
8 changes: 4 additions & 4 deletions src/transform/plugins/imsize/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import {PluginSimple} from 'markdown-it';
import {PluginWithOptions} from 'markdown-it';

import {imageWithSize} from './plugin';
import {ImsizeOptions, imageWithSize} from './plugin';

/**
* Imsize plugin for markdown-it.
* This plugin overloads original image renderer.
* Forked from https://github.com/tatsy/markdown-it-imsize
*/

const imsize: PluginSimple = (md) => {
md.inline.ruler.before('emphasis', 'image', imageWithSize(md));
const imsize: PluginWithOptions<ImsizeOptions> = (md, opts) => {
md.inline.ruler.before('emphasis', 'image', imageWithSize(md, opts));
};

export = imsize;
33 changes: 32 additions & 1 deletion src/transform/plugins/imsize/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import type Token from 'markdown-it/lib/token';
import {ImsizeAttr} from './const';
import {parseImageSize} from './helpers';

export const imageWithSize = (md: MarkdownIt): ParserInline.RuleInline => {
export type ImsizeOptions = {
inlineSizeStyling?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe better is enableInlineStyling

};

export const imageWithSize = (md: MarkdownIt, opts?: ImsizeOptions): ParserInline.RuleInline => {
// eslint-disable-next-line complexity
return (state, silent) => {
if (state.src.charCodeAt(state.pos) !== 0x21 /* ! */) {
Expand Down Expand Up @@ -206,6 +210,33 @@ export const imageWithSize = (md: MarkdownIt): ParserInline.RuleInline => {
if (height !== '') {
token.attrs.push([ImsizeAttr.Height, height]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we still need height and width attributes?

Copy link
Contributor Author

@St1ggy St1ggy Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transform/src/transform/plugins/changelog/index.ts

Lines 77 to 78 in bff7df9

const width = Number(imageToken.attrGet('width'));
const height = Number(imageToken.attrGet('height'));

Alas, that's why we still need it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also noticed complaints about incorrect image appearance when the height was not proportional. However, I did not see any requests for a stretched image or an image with extra frames.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proportions of the image are calculated based on the height and width specified in the markup. If the container is larger than these dimensions, the image will maintain its original height and width. If the width of the container is less than the width of the image, the image's width will adjust to 100% of the container's width and the height will be adjusted proportionally based on the original height and width values from the markup.

}

if (opts?.inlineSizeStyling) {
let style: string | undefined = '';

const widthWithPercent = width.includes('%');
const heightWithPercent = height.includes('%');

if (width !== '') {
const widthString = widthWithPercent ? width : `${width}px`;
style += `width: ${widthString};`;
}

if (height !== '') {
if (width !== '' && !heightWithPercent && !widthWithPercent) {
style += `aspect-ratio: ${width} / ${height};height: auto;`;
state.env.forcedSanitizeCssWhiteList ??= {};
state.env.forcedSanitizeCssWhiteList['aspect-ratio'] = true;
} else {
const heightString = heightWithPercent ? height : `${height}px`;
style += `height: ${heightString};`;
}
}

if (style) {
token.attrs.push([ImsizeAttr.Style, style]);
}
}
}

state.pos = pos;
Expand Down
17 changes: 14 additions & 3 deletions src/transform/sanitize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import cssfilter from 'cssfilter';
import * as cheerio from 'cheerio';
import css from 'css';

import {CssWhiteList} from './typings';

const htmlTags = [
'a',
'abbr',
Expand Down Expand Up @@ -492,8 +494,6 @@ const allowedTags = Array.from(
);
const allowedAttributes = Array.from(new Set([...htmlAttrs, ...svgAttrs, ...yfmHtmlAttrs]));

export type CssWhiteList = {[property: string]: boolean};

export interface SanitizeOptions extends sanitizeHtml.IOptions {
cssWhiteList?: CssWhiteList;
disableStyleSanitizer?: boolean;
Expand Down Expand Up @@ -598,9 +598,20 @@ function sanitizeStyles(html: string, options: SanitizeOptions) {
return styles + content;
}

export default function sanitize(html: string, options?: SanitizeOptions) {
export default function sanitize(
html: string,
options?: SanitizeOptions,
forcedSanitizeCssWhiteList?: CssWhiteList,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I suggest replacing it with an interface that has an additional parameter for uniformity and future expansion of functionality.
    html: string,
    options?: SanitizeOptions,
    additionalOptions?: SanitizeOptions,
  1. Then it will be:
    if (forcedSanitizeCssWhiteList) {
        sanitizeOptions.cssWhiteList = {
            ...sanitizeOptions.cssWhiteList,
            ...additionalOptions.cssWhiteList,
        };
    }

) {
const sanitizeOptions = options || defaultOptions;

if (forcedSanitizeCssWhiteList) {
sanitizeOptions.cssWhiteList = {
...sanitizeOptions.cssWhiteList,
...forcedSanitizeCssWhiteList,
};
}

const needToSanitizeStyles = !(sanitizeOptions.disableStyleSanitizer ?? false);

const modifiedHtml = needToSanitizeStyles ? sanitizeStyles(html, sanitizeOptions) : html;
Expand Down
3 changes: 3 additions & 0 deletions src/transform/typings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export type EnvType<Extras extends {} = {}> = {
assets?: unknown[];
meta?: object;
changelogs?: ChangelogItem[];
forcedSanitizeCssWhiteList?: CssWhiteList;
} & Extras;

export interface MarkdownItPluginOpts {
Expand All @@ -98,3 +99,5 @@ export type MarkdownItPluginCb<T extends {} = {}> = {
export type MarkdownItPreprocessorCb<T extends unknown = {}> = {
(input: string, opts: T & Partial<MarkdownItPluginOpts>, md?: MarkdownIt): string;
};

export type CssWhiteList = {[property: string]: boolean};
20 changes: 20 additions & 0 deletions test/data/imsize/imsize-fixtures.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,26 @@ Coverage. Image
.
<p><img src="test" alt="test" width="100%"></p>
.
.
![test](test =x100%)
.
<p><img src="test" alt="test" height="100%"></p>
.
.
![test](test =100%x100%)
.
<p><img src="test" alt="test" width="100%" height="100%"></p>
.
.
![test](test =100%x200)
.
<p><img src="test" alt="test" width="100%" height="200"></p>
.
.
![test](test =100x100%)
.
<p><img src="test" alt="test" width="100" height="100%"></p>
.

Coverage. Link
.
Expand Down
81 changes: 81 additions & 0 deletions test/data/imsize/imsize-inlineSizeStyling-fixtures.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
Coverage. Image with inlineStyling
.
![test]( x =100x200)
.
<p><img src="x" alt="test" width="100" height="200" style="width: 100px;aspect-ratio: 100 / 200;height: auto;"></p>
.
.
![test]( x =x)
.
<p><img src="x" alt="test"></p>
.
.
![test]( x =100x)
.
<p><img src="x" alt="test" width="100" style="width: 100px;"></p>
.
.
![test]( x =x200)
.
<p><img src="x" alt="test" height="200" style="height: 200px;"></p>
.
.
![test]( x "title" =100x200)
.
<p><img src="x" alt="test" title="title" width="100" height="200" style="width: 100px;aspect-ratio: 100 / 200;height: auto;"></p>
.
.
![test]( x =WxH )
.
<p>![test]( x =WxH )</p>
.
.
![test]( x = 100x200 )
.
<p>![test]( x = 100x200 )</p>
.
.
![test]( x =aaaxbbb )
.
<p>![test]( x =aaaxbbb )</p>
.
.
![test](http://this.is.test.jpg =100x200)
.
<p><img src="http://this.is.test.jpg" alt="test" width="100" height="200" style="width: 100px;aspect-ratio: 100 / 200;height: auto;"></p>
.
.
![test](<x =100x200)
.
<p>![test](&lt;x =100x200)</p>
.
.
![test](<x> =100x200)
.
<p><img src="x" alt="test" width="100" height="200" style="width: 100px;aspect-ratio: 100 / 200;height: auto;"></p>
.
.
![test](test =100%x)
.
<p><img src="test" alt="test" width="100%" style="width: 100%;"></p>
.
.
![test](test =x100%)
.
<p><img src="test" alt="test" height="100%" style="height: 100%;"></p>
.
.
![test](test =100%x100%)
.
<p><img src="test" alt="test" width="100%" height="100%" style="width: 100%;height: 100%;"></p>
.
.
![test](test =100%x200)
.
<p><img src="test" alt="test" width="100%" height="200" style="width: 100%;height: 200px;"></p>
.
.
![test](test =100x100%)
.
<p><img src="test" alt="test" width="100" height="100%" style="width: 100px;height: 100%;"></p>
.
10 changes: 10 additions & 0 deletions test/imsize.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,13 @@ describe('imsize', () => {

generate(path.join(__dirname, 'data/imsize/imsize-fixtures.txt'), md);
});

describe('imsize with inlineStyling', () => {
const md = new MarkdownIt({
html: true,
linkify: false,
typographer: false,
}).use(imsize, {inlineSizeStyling: true});

generate(path.join(__dirname, 'data/imsize/imsize-inlineSizeStyling-fixtures.txt'), md);
});
Loading