Patch #40939
openAdd "underline" button to jsToolbar for CommonMark Markdown formatting
Added by Yasu Saku about 1 month ago. Updated 28 days ago.
0%
Description
The Textile and Markdown text formatting have an "underline" button in the jsToolbar.
However, the CommonMark Markdown text format does not.
Also, CommonMark Markdown supports HTML tags.
So I created a patch to insert `<ins></ins>` tags.
Files
patch.diff (568 Bytes) patch.diff | Yasu Saku, 2024-07-06 14:03 | ||
40939-v2.patch (1.06 KB) 40939-v2.patch | Go MAEDA, 2024-07-07 07:55 | ||
0001-Change-comment-to-u-for-Underline-button.patch (907 Bytes) 0001-Change-comment-to-u-for-Underline-button.patch | Go MAEDA, 2024-07-19 09:56 | ||
0002-Add-Underline-button-to-jsToolbar-for-CommonMark-Mar.patch (1.47 KB) 0002-Add-Underline-button-to-jsToolbar-for-CommonMark-Mar.patch | Go MAEDA, 2024-07-19 09:56 |
Updated by Go MAEDA about 1 month ago
- File 40939-v2.patch 40939-v2.patch added
Thank you for the proposal to add an "Underline" button to the CommonMark Markdown toolbar.
However, I think it is better to use <u>
instead of <ins>
, as in Redcarpet-based Markdown. This is because <ins>
is semantically intended to represent inserted text, so using <ins>
only to express underline is not semantically appropriate.
I have updated the patch to use the <u>
element accordingly.
Updated by Yasu Saku about 1 month ago
Go MAEDA wrote in #note-1:
Thank you for the proposal to add an "Underline" button to the CommonMark Markdown toolbar.
However, I think it is better to use
<u>
instead of<ins>
, as in Redcarpet-based Markdown. This is because<ins>
is semantically intended to represent inserted text, so using<ins>
only to express underline is not semantically appropriate.I have updated the patch to use the
<u>
element accordingly.
Thank you for the correction and explanation.
But, I am a little concerned about the `ins` in the comments, including other text formatting. Why?
Updated by Go MAEDA 30 days ago
I investigated why the comment // ins
is used for the "Underline" button in markdown.js.
First, in Textile, which was the only available formatter before Redcarpet-based Markdown was implemented in Redmine 2.5.0, the <ins>
tag is used to represent underlined text. Therefore, it makes sense for the comment // ins
to be used for the "Underline" button in textile.js (source:trunk/app/assets/javascripts/jstoolbar/textile.js@22937#L29).
However, in Markdown based on Redcarpet, the <u>
tag is used for underlined text. Despite this, the comment in markdown.js still says // ins
(source:trunk/app/assets/javascripts/jstoolbar/markdown.js@22937#L29). It would be more appropriate for it to say // u
. It is likely that when the underline button was added to the Markdown toolbar in r17385, the comment // ins
from textile.js was copied over without being changed.
Therefore, I suggest proceeding as follows:
- For the CommonMark Markdown toolbar, follow the implementation of Redcarpet-based Markdown and use the
<u>
tag to represent underlined text, as I proposed in 40939-v2.patch. - Change the comment
// ins
in 40939-v2.patch to// u
. - Change the existing comments in markdown.js to
// u
to match the HTML output by Redcarpet.
Updated by Go MAEDA 29 days ago
- File 0001-Change-comment-to-u-for-Underline-button.patch 0001-Change-comment-to-u-for-Underline-button.patch added
- File 0002-Add-Underline-button-to-jsToolbar-for-CommonMark-Mar.patch 0002-Add-Underline-button-to-jsToolbar-for-CommonMark-Mar.patch added
Following the proposal made in #note-3, I have updated the patches.
1. The first patch corrects the comment in the Redcarpet-based Markdown underline button code to // u
. This is because the final output HTML uses the `<u>` tag to achieve the underline effect.
2. The second patch adds an underline button to the CommonMark Markdown toolbar. This is a slightly modified version of the patch provided by Yasu Saku.
Updated by Yasu Saku 28 days ago
Thank you Mr. Go MAEDA.
I have understood the historical context in which the //ins
comment was added.
In addition, I think it would be better to change jsToolBar.prototype.elements.ins
to jsToolBar.prototype.elements.u
.
What do you think?
diff --git a/app/assets/javascripts/jstoolbar/common_mark.js b/app/assets/javascripts/jstoolbar/common_mark.js
index f9e5ba7..5c3627a 100644
--- a/app/assets/javascripts/jstoolbar/common_mark.js
+++ b/app/assets/javascripts/jstoolbar/common_mark.js
@@ -26,6 +26,16 @@ jsToolBar.prototype.elements.em = {
}
}
+// u
+jsToolBar.prototype.elements.u = {
+ type: 'button',
+ title: 'Underline',
+ shortcut: 'u',
+ fn: {
+ wiki: function() { this.singleTag('<u>', '</u>') }
+ }
+}
+
// del
jsToolBar.prototype.elements.del = {
type: 'button',
diff --git a/app/assets/stylesheets/jstoolbar.css b/app/assets/stylesheets/jstoolbar.css
index 6870261..38c70c1 100644
--- a/app/assets/stylesheets/jstoolbar.css
+++ b/app/assets/stylesheets/jstoolbar.css
@@ -113,6 +113,9 @@
.jstb_ins {
background-image: url(/jstoolbar/bt_ins.png);
}
+.jstb_u {
+ background-image: url(/jstoolbar/bt_ins.png);
+}
.jstb_del {
background-image: url(/jstoolbar/bt_del.png);
}