FoundKeyGang/FoundKey
FoundKeyGang
/
FoundKey
7
19
Fork 22
Code Issues 37 Pull Requests 4 Releases Activity

Updates mfm-js and adds position, scale, fg and bg function to mfm render #385

Manually merged
Johann150 merged 5 commits from puniko/FoundKey:update-mfm-core into main 2023-05-27 10:14:40 +00:00
Conversation 2 Commits 5 Files Changed 6 +62 -34
6 changed files with 62 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/backend/package.json
View File

@ -67,7 +67,7 @@
"koa-send": "5.0.1",
"koa-slow": "2.1.0",
"koa-views": "7.0.2",
"mfm-js": "0.22.1",
"mfm-js": "0.23.3",
"mime-types": "2.1.35",
"mocha": "10.2.0",
"multer": "1.4.5-lts.1",

2
packages/backend/src/server/api/endpoints/i/update.ts
View File

@ -178,7 +178,7 @@ export default define(meta, paramDef, async (ps, _user, token) => {
const newDescription = profileUpdates.description === undefined ? profile.description : profileUpdates.description;
if (newName != null) {
const tokens = mfm.parsePlain(newName);
const tokens = mfm.parseSimple(newName);
emojis = emojis.concat(extractCustomEmojisFromMfm(tokens!));
}

2
packages/client/package.json
View File

@ -34,7 +34,7 @@
"json5": "2.2.1",
"katex": "0.16.0",
"matter-js": "0.18.0",
"mfm-js": "0.22.1",
"mfm-js": "0.23.3",
"photoswipe": "5.2.8",
"prismjs": "1.28.0",
"punycode": "2.1.1",

72
packages/client/src/components/mfm.ts
View File

@ -10,7 +10,6 @@ import MkSearch from '@/components/mfm-search.vue';
import MkSparkle from '@/components/sparkle.vue';
import MkA from '@/components/global/a.vue';
import { host } from '@/config';
import { MFM_TAGS } from '@/scripts/mfm-tags';
export default defineComponent({
props: {
@ -37,12 +36,16 @@ export default defineComponent({
type: Boolean,
default: true,
},
rootScale: {
type: Number,
default: 1
}
},
render() {
if (this.text == null || this.text === '') return;
const ast = (this.plain ? mfm.parsePlain : mfm.parse)(this.text, { fnNameList: MFM_TAGS });
const ast = (this.plain ? mfm.parseSimple : mfm.parse)(this.text);
const validTime = (t: string | true) => {
if (typeof t !== 'string') return null;
@ -50,7 +53,7 @@ export default defineComponent({
return t.match(/^[0-9.]+s$/) ? t : null;
};
const genEl = (ast: mfm.MfmNode[]) => ast.map((token): VNode | VNode[] => {
const genEl = (ast: mfm.MfmNode[], scale: Number) => ast.map((token): VNode | VNode[] => {
Johann150 marked this conversation as resolved
Johann150 commented 2023-05-21 18:58:36 +00:00
Review
Copy Link

I don't understand what this scale parameter is for, it doesn't seem to be read at any point and having to carry it through is kind of annoying.

I don't understand what this `scale` parameter is for, it doesn't seem to be read at any point and having to carry it through is kind of annoying.
switch (token.type) {
case 'text': {
const text = token.props.text.replace(/(\r\n|\n|\r)/g, '\n');
@ -69,17 +72,17 @@ export default defineComponent({
}
case 'bold': {
return h('b', genEl(token.children));
return h('b', genEl(token.children, scale));
}
case 'strike': {
return h('del', genEl(token.children));
return h('del', genEl(token.children, scale));
}
case 'italic': {
return h('i', {
style: 'font-style: oblique;',
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'fn': {
@ -139,18 +142,18 @@ export default defineComponent({
}
case 'x2': {
return h('span', {
class: 'mfm-x2',
}, genEl(token.children));
class: 'mfm-x2'
}, genEl(token.children, scale * 2));
}
case 'x3': {
return h('span', {
class: 'mfm-x3',
}, genEl(token.children));
class: 'mfm-x3'
}, genEl(token.children, scale * 3));
}
case 'x4': {
return h('span', {
class: 'mfm-x4',
}, genEl(token.children));
class: 'mfm-x4'
}, genEl(token.children, scale * 4));
}
case 'font': {
const family =
@ -167,7 +170,7 @@ export default defineComponent({
case 'blur': {
return h('span', {
class: '_mfm_blur_',
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'rainbow': {
const speed = validTime(token.props.args.speed) || '1s';
@ -176,35 +179,60 @@ export default defineComponent({
}
case 'sparkle': {
if (!this.$store.state.animatedMfm) {
return genEl(token.children);
return genEl(token.children, scale);
}
return h(MkSparkle, {}, genEl(token.children));
return h(MkSparkle, {}, genEl(token.children, scale));
}
case 'rotate': {
const degrees = (typeof token.props.args.deg === 'string' ? parseInt(token.props.args.deg) : null) || '90';
style = `transform: rotate(${degrees}deg); transform-origin: center center;`;
break;
}
case 'position': {
const x = parseFloat(token.props.args.x ?? '0');
const y = parseFloat(token.props.args.y ?? '0');
style = `transform: translateX(${x}em) translateY(${y}em);`;
break;
}
case 'scale': {
const x = Math.min(parseFloat(token.props.args.x ?? '1'), 5);
const y = Math.min(parseFloat(token.props.args.y ?? '1'), 5);
style = `transform: scale(${x}, ${y});`;
Johann150 marked this conversation as resolved
Johann150 commented 2023-05-21 19:04:34 +00:00
Review
Copy Link

Not really happy with this because this allows to bypass the limitations that were created for x2, x3 and x4 to not allow someone else to insert MFM resulting in people having HUGE things on their timeline that they have to scroll by for hours.

My idea to fix it would be to use CSS and add it to these existing rules for x2, x3 and x4 functions: 7a94e9f2d5/packages/client/src/components/global/misskey-flavored-markdown.vue (L33-L57) (I'm not entirely sure if that works because those rules use font-size while you used transform: scale.)

Not really happy with this because this allows to bypass the limitations that were created for `x2`, `x3` and `x4` to not allow someone else to insert MFM resulting in people having HUGE things on their timeline that they have to scroll by for hours. My idea to fix it would be to use CSS and add it to these existing rules for `x2`, `x3` and `x4` functions: https://akkoma.dev/FoundKeyGang/FoundKey/src/commit/7a94e9f2d5804b6331272cc5d9db2ff12d8fde05/packages/client/src/components/global/misskey-flavored-markdown.vue#L33-L57 (I'm not entirely sure if that works because those rules use `font-size` while you used `transform: scale`.)
freeplay commented 2023-05-25 23:53:34 +00:00
Review
Copy Link

Transform's don't affect the content height

If you don't want it to overflow into the replies, then you should add overflow: hidden or clip to the note (unless you already have)

Transform's don't affect the content height If you don't want it to overflow into the replies, then you should add overflow: hidden or clip to the note (unless you already have)
👀 1
Johann150 commented 2023-05-27 09:33:05 +00:00
Review
Copy Link

Thanks for pointing this out. I guess this is not an issue then.

Thanks for pointing this out. I guess this is not an issue then.
scale = scale * Math.max(x, y);
break;
}
case 'fg': {
let color = token.props.args.color;
if (!/^[0-9a-f]{3,6}$/i.test(color)) color = 'f00';
Johann150 marked this conversation as resolved
Johann150 commented 2023-05-21 19:13:23 +00:00
Review
Copy Link

I think this should have an explicit check for undefined (if the argument is not specified at all) and true (if the argument is specified without value).

All values are coerced to strings, so omitting it or passing undefined causes test() to search for the string "undefined", which is rarely what you want.
-- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test

I also think the regex is incorrect since {3,6} means "3 to 6 repetitions" and not "3 or 6 repetitions". I.e. this would allow 4 and 5 character colour codes which I think is not understood by browsers.

Putting the default if it is an invalid colour seems a bit weird to me but I guess if that's what Misskey does we should probably do the same.

I think this should have an explicit check for `undefined` (if the argument is not specified at all) and `true` (if the argument is specified without value). > All values are coerced to strings, so omitting it or passing `undefined` causes `test()` to search for the string `"undefined"`, which is rarely what you want. -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test I also think the regex is incorrect since `{3,6}` means "3 to 6 repetitions" and not "3 or 6 repetitions". I.e. this would allow 4 and 5 character colour codes which I think is not understood by browsers. Putting the default if it is an invalid colour seems a bit weird to me but I guess if that's what Misskey does we should probably do the same.
toast commented 2023-05-26 18:00:42 +00:00
Review
Copy Link

Recommended alternative regex: /^[0-9a-fA-F]{3}([0-9a-fA-F]{3})?$/ or so.

Recommended alternative regex: `/^[0-9a-fA-F]{3}([0-9a-fA-F]{3})?$/` or so.
👍 1
Johann150 commented 2023-05-26 19:24:20 +00:00
Review
Copy Link

or /^([0-9a-f]{3}){1,2}$/i stolen from this gist

or `/^([0-9a-f]{3}){1,2}$/i` stolen from [this gist](https://gist.github.com/olmokramer/82ccce673f86db7cda5e)
style = `color: #${color};`;
break;
}
case 'bg': {
let color = token.props.args.color;
if (!/^[0-9a-f]{3,6}$/i.test(color)) color = 'f00';
Johann150 marked this conversation as resolved
Johann150 commented 2023-05-21 19:14:09 +00:00
Review
Copy Link

The comment for fg applies here as well of course.

The comment for `fg` applies here as well of course.
style = `background-color: #${color};`;
break;
}
}
if (style == null) {
return h('span', {}, ['$[', token.props.name, ' ', ...genEl(token.children), ']']);
return h('span', {}, ['$[', token.props.name, ' ', ...genEl(token.children, scale), ']']);
} else {
return h('span', {
style: 'display: inline-block;' + style,
}, genEl(token.children));
}, genEl(token.children, scale));
}
}
case 'small': {
return h('small', {
class: '_mfm_small_'
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'center': {
return h('div', {
style: 'text-align:center;',
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'url': {
@ -220,7 +248,7 @@ export default defineComponent({
key: Math.random(),
url: token.props.url,
rel: 'nofollow noopener',
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'mention': {
@ -258,7 +286,7 @@ export default defineComponent({
case 'quote': {
return h(this.nowrap ? 'span' : 'div', {
class: 'quote',
}, genEl(token.children));
}, genEl(token.children, scale));
}
case 'emojiCode': {
@ -311,6 +339,6 @@ export default defineComponent({
}).flat();
// Parse ast to DOM
return h('span', genEl(ast));
return h('span', genEl(ast, this.rootScale ?? 1));
},
});

2
packages/client/src/scripts/mfm-tags.ts
View File

@ -1 +1 @@
export const MFM_TAGS = ['tada', 'jelly', 'twitch', 'shake', 'spin', 'jump', 'bounce', 'flip', 'x2', 'x3', 'x4', 'font', 'blur', 'rainbow', 'sparkle', 'rotate'];
export const MFM_TAGS = ['tada', 'jelly', 'twitch', 'shake', 'spin', 'jump', 'bounce', 'flip', 'x2', 'x3', 'x4', 'font', 'blur', 'rainbow', 'sparkle', 'rotate', 'position', 'scale', 'fg', 'bg'];

16
yarn.lock
View File

@ -3750,7 +3750,7 @@ __metadata:
koa-send: 5.0.1
koa-slow: 2.1.0
koa-views: 7.0.2
mfm-js: 0.22.1
mfm-js: 0.23.3
mime-types: 2.1.35
mocha: 10.2.0
multer: 1.4.5-lts.1
@ -4720,7 +4720,7 @@ __metadata:
json5: 2.2.1
katex: 0.16.0
matter-js: 0.18.0
mfm-js: 0.22.1
mfm-js: 0.23.3
photoswipe: 5.2.8
prismjs: 1.28.0
punycode: 2.1.1
@ -11671,12 +11671,12 @@ __metadata:
languageName: node
linkType: hard
"mfm-js@npm:0.22.1":
version: 0.22.1
resolution: "mfm-js@npm:0.22.1"
"mfm-js@npm:0.23.3":
version: 0.23.3
resolution: "mfm-js@npm:0.23.3"
dependencies:
twemoji-parser: 14.0.x
checksum: 6d9756c7bd8abf6462fb6403de4656f607a83839eb6b66a05b10eddcd201b5f78f5fe3d0df029936546143fd9cbf112e8369287aed32026e50bb03ce89b4c4f8
twemoji-parser: 14.0.0
checksum: 7079f80a53a9afc8599333f3256fb18a6bf7c01102a2f8f2be657843726a34835e2af34e26bc5b27e45b217fb2f120c0d3006e9fab2a972c845e9f7361e3cc1b
languageName: node
linkType: hard
@ -16723,7 +16723,7 @@ __metadata:
languageName: node
linkType: hard
"twemoji-parser@npm:14.0.0, twemoji-parser@npm:14.0.x":
"twemoji-parser@npm:14.0.0":
version: 14.0.0
resolution: "twemoji-parser@npm:14.0.0"
checksum: 8eede69cf71f94735de7b6fddf5dfbfe3cb2e01baefc3201360984ccc97cfc659f206c8f73bd1405a2282779af3b79a8c9bed3864c672e15e2dc6f8ce4810452