-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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.
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: GitHub style callouts #2487
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
82d428f
to
913112f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little changes I proposed.
BTW, the important icon
is cute.
// 0: Match "[!TIP] My Title" | ||
// 1: Mark "[!TIP]" | ||
// 2: Type "TIP" | ||
tokens[0].raw.match(/^(\[!(\w+)\])/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tokens[0].raw.match(/^(\[!(\w+)\])/); | |
tokens[0].raw.match(/^(\[!(CAUTION|IMPORTANT|NOTE|TIP|WARNING)\])/); |
I think we need strict match the callout tags in case of users use something like ![DUCK]
in some reason.
And we have no callout class
to handle it either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional. :)
Callouts (or "admonitions" or "alerts") are found in many other apps and the list of callouts supported is often much larger than what GitHub supports. By capturing the string contained with the brackets ([!STRING]
) and using it as a separate styling-only class (<div class="callout string">
) it simplified adding new callouts for third parties.
For example:
> [!BUG]
> This is a bug callout
.callout.bug {
--callout-bg: orange;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. so it means the ![words]
is a callout
syntax hook (I think I can call it as an reserved word/block
) in docsify and users could do the extension.Meanwhile, it will disable to allow user uses a ![not-callout]
as a plain start of blockquote.
> [!question] Should we mention it in `UI-KIT`?
> > [!advantage] users can understand there is a way to do extension for callout block (so am I xD ).
> > > [!example] or we could also add an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
The .callout
class handles all of the base styling. The additional class on the callout element (.caution
, .important
, .note
, etc.) is used to adjust styles like colors and icons. To extend the system to support the example you provided above they would simply need to create CSS declarations for each:
callout.question {
--callout-bg: red;
--callout-color: white;
}
I can add a note about custom classes to the docs along with a note explaining that in most cases custom classes will not work outside of Docsify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add a note about custom classes to the docs along with a note explaining that in most cases custom classes will not work outside of Docsify.
Hi, @jhildenbiddle Do you still need to modify the document? If not, then you can merge this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sy-records Yep. Haven't had a chance to complete the updated docs, but I'll have that in the next or two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other enhancements I'd like to make is the ability to define localized labels. I make a conscious decision to not implement this initially for a few reasons, but after thinking it over for a bit I've decided it makes sense to offer this feature.
The implementation will follow the same pattern we've used elsewhere in our configuration. The default behavior will be to not render text labels (as shown in screenshots above):
window.$docsify = {
// Default: do not render text labels
callouts: {},
};
If users want to render text labels automatically as is done in markdown environments like GitHub and Obsidian, they can define them as part of their configuration:
window.$docsify = {
// Render same label for all route paths
callouts: {
caution: 'Caution',
important: 'Important',
note: 'Note',
tip: 'Tip',
warning: 'Warning',
},
};
window.$docsify = {
// Render localized labels based on route paths
callouts: {
caution: {
'/es/': 'Precaución',
'/de-de/': 'Vorsicht',
'/ru-ru/': 'Осторожность',
'/zh-cn/': '警告',
'/': 'Caution',
},
// ...
},
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
----- @jhildenbiddle , Currently, we simply add a icon per callout config. That looks good to me.
I don't propose that we need introduce a label
into docsify with the i18n support either:
- We need extract the
content
from the[!HERE]
and match it to the$.callouts
config by path and by content.
i.e. if user has the![example]
, they absolutely want to config it like this:
window.$docsify = {
// Render same label for all route paths
callouts: {
example: {
'/zh-cn/': '例子',
'/': 'Example',
},
},
It makes the config more complex and messy, they need config options size is: i18n-paths * callouts
.
IMO, the icon
already has the info
, we don't need mention it specially with a label
.
In a common sense, a light lamp is tips
, a i
is info/note, a red thing is forbidden/attention, a yellow triangle is warning. user can simply use it as it default meanings.
The only different thing is we introduce some icon
more docsify style (such as important
is a cute star.). thats fine since we don't fu*k any things up.
If user do wanna enrich the label
in their multi lang sites for each icon, they can simply add something like this:
> [!IMPORTANT] **Important** in EN site.
> [!IMPORTANT] **重要** in ZH site.
Besides.
We don't need stuck in the [!IMPORTANT] is an IMPORTANT icon
.
User can treat the [!SOMEKEY]
just a tag
or a snytax
that docsify would provide some callout icons
.
Which means, user could just know something like:
If you config the
[!TIP]
, docsify will give a lamp.
Then, they can append all the extra info for the icon
they want or fully "rename" them.
No matter user is an EN speaker or not and how they categorize the icons in their site with their customized icon and docsify builtin icons.
> [!IMPORTANT] **Bonus Time** When you see the Star icon, it means we brings you bonus surprise now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some doc details for the reserved callout syntax and the extension. others LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much @jhildenbiddle, this is a valuable addition to the Docsify Markdown ecosystem!
913112f
to
6cf5f36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jhildenbiddle, this is an awesome feature! I've been missing this, and thus I have been using a docsify-plugin-flexible-alerts
to enable a similar feature although not exactly the same as GitHub's syntax.
A couple questions:
-
Does this need to be in core? Or can it be an official plugin?
-
Can we please follow the GitHub behavior?
More on question 2:
For example in GitHub we can input this markdown:
> [!Note]
> This is a Note
> [!CAUTION]
> This is a cautionary thing (case of the header doesn't matter)
and this is the result:
Note
This is a Note
Caution
This is a cautionary thing (case of the header doesn't matter)
Note that I did not have to input a redundant additional **Note**
or **Caution**
, as GitHub will automatically input the text for the callout.
On the otherhand, based on what I see in the OP screenshots, the callout names will not be automatically added, and the Docsify user has to redundantly input them like this:
> [!CAUTION]
> **Caution** callouts communicate negative potential consequences of an action.
The result on GitHub is this:
Caution
Caution callouts communicate negative potential consequences of an action.
which as we can see results in "caution" being there twice, which means rendering will differ between Docsify and GitHub.
Hiiii @trusktr Joe.
I think the behavior is totally acceptable.
i.e. All is EN, and github's callout icon supports only EN. it seems duplicated. Caution Caution callouts communicate negative potential consequences of an action.
Caution 非常重要 这里是一条你不能错过的信息。 IMHO, It is worse than only simply support a icon. |
You both have valid points. I believe the current design and proposed custom label feature (described here) addresses both of them. For context, understand that a large number of people are unhappy with GitHub's English-centric callout design (see https://github.com/orgs/community/discussions/16925#discussioncomment-9861713). GitHub callouts always display a label and that label is always in English. There is no way to override this, which can be problematic when authoring markdown non-English languages. The Docsify callout design addresses GitHub's English-centric design by rendering only an icon without a label by default. This allows users to easy customize callout labels as they prefer directly in their markdown: ![]() Markdown: <!-- No label, single line -->
> [!caution]
> This is the callout text.
<!-- EN label, single line -->
> [!caution] **Caution:** This is the callout text.
<!-- EN label, new line -->
> [!caution]
>
> **Caution**
>
> This is the callout text.
<!-- ZH label, new line -->
> [!caution]
>
> **非常重要 这里是**
>
> This is the callout text. However, I believe @trusktr is correct that there will be users who expect Docsify to render callout labels as GitHub does. Users who manually add callout labels in their markdown to "fix" the problem in Docsify may also be unhappy when duplicate titles are rendered on GitHub. This is why I would like to optionally render titles and provide the ability to define localized labels as part of the Docsify config here. Per @Koooooo-7's preference, the default configuration will render icons only: window.$docsify = {
// Default: do not render callout titles
callouts: {},
}; Per @trusktr's preference, specifying callout labels in English will render callout labels as seen on GitHub and avoid duplicate titles when markdown is rendered on GitHub: window.$docsify = {
// Render EN callout titles for all route paths
callouts: {
caution: 'Caution',
important: 'Important',
note: 'Note',
tip: 'Tip',
warning: 'Warning',
},
}; For sites offering localized documentation, callout labels can be defined based on the route path: window.$docsify = {
// Render localized labels based on route paths
callouts: {
caution: {
'/es/': 'Precaución',
'/de-de/': 'Vorsicht',
'/ru-ru/': 'Осторожность',
'/zh-cn/': '警告',
'/': 'Caution',
},
important: {
// ...
},
note: {
// ...
},
tip: {
// ...
},
warning: {
// ...
},
},
}; NOTE: After this PR is merged, it is my intention to recommend GitHub consider a similar "icons only be default" design to address the concerns with their current EN-centric design. I would like to link to our own documentation (once completed) as a reference. |
@jhildenbiddle --- I keep my point that we could just provide a
I understand the |
Agreed. This is why I opted for the "icon-only" presentation initially and propose it remain the default presentation for Docsify. I believe it is better than GitHub's implementation because it addresses internationalization issues inherent in GitHub's implementation and gives users control over if/how callout titles are rendered. However, it is also true the icon-only presentation is unique to Docsify and not how other popular markdown environments render the same callout syntax. One way to "fix" the inconsistency in Docsify is to add text labels in markdown, however this will produce duplicate labels when rendered on GitHub and other environments that support the Caution CAUTION This is some text Missing or duplicate labels may not matter to some but they definitely will matter to others. Hence the label option.
Agreed on the "if they want to do so, they can" part. The label configuration option provides a way of rendering callout labels that also prevents duplicate labels from being rendered outside of Docsify (see example above). This issue is created by our decision to have a different default presentation (icon-only) than other markdown environments that support the same syntax. If we instead chose to render callous labels by default as other markdown environments do, we wouldn't have to worry about duplicate callout labels when rendering callout markdown outside of Docsify.
This is about content consistency, not style consistency. The issue people have with GitHub's callout implementation is that it introduces potentially unwanted content in the form of English-only labels without the ability to modify them. I think this is a mistake, as do many others who have shared their opinions in the GitHub callout discussion. Regardless and for better or worse, other markdown environments that offer native support for the same syntax (e.g., Obsidian, Typora, VitePress) also render callout labels by default. In order for Docsify to render callouts consistently across environments, we need to provide an option to render callout labels as other environments do. This matters because markdown is a portable documentation format and it is therefore not uncommon for people/teams to edit markdown content in different environments. For example, I frequently edit Docsify markdown in Typora and I know first-hand of teams that have non-technical team members edit Docsify markdown content directly on GitHub. As far as styles go, Docsify has its own callout style (icons, colors, border, backgrounds, etc.) just as other markdown environments do. This is not unexpected as it is very much a benefit/feature of markdown's portability.
I would love to remove non-standard, Docsify-only syntax from Docsify because the more of it we offer the less portable Docsify markdown becomes. Unfortunately, there are some features that markdown does not offer and extended syntaxes to support them have not been standardized so we're left to invent our own sometimes. IMHO, these non-standard, Docsify-only markdown extensions are a necessary evil, not a pattern to continue if we can help it. In the case of callouts, a standardized syntax did not exist back in 2017 when Docsify v4 was released so the custom "important" ( |
--- @trusktr
Agreed. and I have more deep realization of the github influence now.
Jump in. :)
Good idea to more customized the callout. |
I would love to get this merged, but there's one question we haven't chatted about yet:
I personally wish that if there's something we can do as a plugin, that we do it as a plugin, so we can keep core less complicated and smaller, and people can expand the features they need. Is there anything missing that we need to add to the plugin API before this can be a plugin? If so, we can add that here in the same PR too (or separate if you prefer). As for the default options, I can live with no call-out text being the default, and I can easily add the options to my config to make it follow GitHub's style. We can also make this prominent in the docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to address of this can be in a plugin (previous comment)
Summary
Markdown
Screenshots
Related issue, if any:
#2486
#1588
#483
What kind of change does this PR introduce?
Feature
Docs
For any code change,
Does this PR introduce a breaking change?
No
Tested in the following browsers: