Skip to content

Conversation

@jamliaoo
Copy link
Contributor

@jamliaoo jamliaoo commented Nov 17, 2024

Description

  • 新增 marked extension,可以使用 github alert 的語法
  • 調整 index.css,引入 tailwind style,同步 web editor style
  • 調整 Content.js,使用和 web editor 相同的 element 與 attribute,確保樣式一至

Discussion

  • 樣式部分使用了 tailwind,但若把 tailwind 樣式全部包進去的話,css 會變 2kb 比 js 還大。而且會影響到原本的數學題的樣式,尚不確定原因。所以目前解法是先只引入 tailwind utilities,但樣式會少掉 border,並且也會和編輯器的不太一至,待討論。
  • 我原本以為打包階段 tailwind 會自動移除沒用到的 class,但好像沒生效,不確定是不是因為 cargo 那邊沒有設定

完整引入 tailwind css:css 2kb
full-tailwind-css

只引入 utilities:css 763B
utilities-tailwind-css

11/18 討論結論

  • css 2kb 的大小 ok,tailwind 相關的東西都先加上
  • tailwind 如果有不一至的話,可以先同步兩邊的 config

@jamliaoo jamliaoo requested a review from WendellLiu November 17, 2024 16:10
@@ -1 +1 @@
window.contentConfig = {"title":"Access8Math","latexDelimiter":"bracket","documentFormat":"block","documentDisplay":"markdown","sourceText":"## Question 1\n\n1. \\({{\\left( -3 \\right)}^{3}}\\)之值為何?\n(A) \\(-27\\)\n(B) \\(-9\\)\n(C) \\(9\\)\n(D) \\(27\\)\n2. aewf",documentColor: "dark"} No newline at end of file
window.contentConfig = {"title":"Access8Math","latexDelimiter":"bracket","documentFormat":"block","documentDisplay":"markdown","documentColor": "light","sourceText":"## Question 1\n\n1. \\({{\\left( -3 \\right)}^{3}}\\)之值為何?\n(A) \\(-27\\)\n(B) \\(-9\\)\n(C) \\(9\\)\n(D) \\(27\\)\n2. aewf\n ## Alert Example\n\n> [!NOTE]\n> Highlights information that users should take into account, even when skimming.\n\n> [!TIP]\n> Optional information to help a user be more successful.\n\n> [!IMPORTANT]\n> Crucial information necessary for users to succeed.\n\n> [!WARNING]\n> Critical content demanding immediate user attention due to potential risks.\n\n> [!CAUTION]\n> Negative potential consequences of an action."} No newline at end of file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • 新增 alert 的 sample 文字
  • 把 sourceText 放到最後面,方便看其他 property 的值

@jamliaoo
Copy link
Contributor Author

  • 調整 index.css,引入 tailwind style,同步 web editor style
  • 調整 Content.js,使用和 web editor 相同的 element 與 attribute,確保樣式一至

Screenshot 2024-11-21 at 4 02 20 PM

Screenshot 2024-11-21 at 4 36 47 PM

Comment on lines 68 to 105
if (matchedVariant) {
const {
type: variantType,
icon,
title = ucfirst(variantType),
titleClassName = `${className}-title`,
} = matchedVariant;
const typeRegexp = new RegExp(createSyntaxPattern(variantType));

Object.assign(token, {
type: 'alert',
meta: {
className,
variant: variantType,
icon,
title,
titleClassName,
},
});

const firstLine = token.tokens?.[0];
const firstLineText = firstLine.raw?.replace(typeRegexp, '').trim();

if (firstLineText) {
const patternToken = firstLine.tokens[0];

Object.assign(patternToken, {
raw: patternToken.raw.replace(typeRegexp, ''),
text: patternToken.text.replace(typeRegexp, ''),
});

if (firstLine.tokens[1]?.type === 'br') {
firstLine.tokens.splice(1, 1);
}
} else {
token.tokens?.shift();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we avoid "nested if-block" and "else"?

if (!matchedVariant) return;
// some codes

if (!firstLineText) token.tokens?.shift();
// some codes

if (firstLine.tokens[1]?.type === 'br') {
  firstLine.tokens.splice(1, 1);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, it’s a great suggestion
aee4baa

Comment on lines 89 to 106
const firstLine = token.tokens?.[0];
const firstLineText = firstLine.raw?.replace(typeRegexp, '').trim();

if (!firstLineText) {
token.tokens?.shift();
return;
}

const patternToken = firstLine.tokens[0];

Object.assign(patternToken, {
raw: patternToken.raw.replace(typeRegexp, ''),
text: patternToken.text.replace(typeRegexp, ''),
});

if (firstLine.tokens[1]?.type === 'br') {
firstLine.tokens.splice(1, 1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know mutating is the pattern the official demos, so it's okay to me to mutate directly on token. However, could you avoid mutating the children of token by accessing and mutating? For readability, please consider reassgin instead of deep mutation.

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’ve updated the code to avoid deep mutations on children of token and switched to reassigning instead. I think it’s more readable now. Let me know if you have any additional suggestions!
9f63a68

Comment on lines 34 to 39
return Object.values(
[...defaultAlertVariant, ...variants].reduce((map, item) => {
map[item.type] = item;
return map;
}, {}),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. please do not mutate the argument in a reduce.
  2. why do we need a reduce but turn out we only return the values of the map from reduce? it looks like it is equivalent to [...defaultAlertVariant, ...variants]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, the handling of this function isn’t ideal. I originally wanted to make it similar to the marked alert package by allowing external options, but looking at it now, it feels unnecessary and just adds complexity. So, I’ve removed the function for now. Let me know if you’ve got other thoughts!
ded5016

@jamliaoo jamliaoo merged commit db9b9d1 into main Jan 6, 2025
@jamliaoo jamliaoo deleted the feat/marked-alert branch January 6, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants