CSSを活用して<strong>と色付き文字を整理する

[dep5]

PR対象

• ドキュメント(md、ヘルプファイル等)

カテゴリ

• 改善

PR の背景

#1505 (ヘルプHTMLの修正)の見直しを中心に気づいたところを修正しています。

仕様・動作説明

1. <strong>を<span>に変更し
   <b>から<strong>に変更する前のこの時点で<strong>だった22か所を<strong>に戻しています。
   強調箇所が黒太文字に変わります

2. </font>の変更漏れを直しています

3. 文字の色指定をstyle属性からclass属性に変更しています。
   CSSでのデザインを行いやすくするのが目的です。
   重要性を表す色指定は必要ならば改めて別のclass名またはタグを当てます。

4. CSS中の必要なセミコロンが無いミスを修正します。
   合わせてスペースなどCSSの表記を整理しました。

5. CSSとHTMLのどちらにも同じstyle指定がある<table>タグについて、CSSに一本化します。

6. タグの閉じかっこの重複を削除します。CSSを活用して<strong>と色付き文字を整理する #1895 (comment)

PR の影響範囲

強調箇所が黒太文字に変わります。
後は見た目に変更はないと思います。

テスト内容

数ページブラウザーで確認しています。

関連 issue, PR

#1505
#1889
#1903

参考資料

[dep5]

<spanと</span>でGrepした時の結果が違うのは
HLP000145で閉じタグのない<span>が使われているからです。

<span style="color: blue">などもclassに変えようかと思いましたが、保留にしています。
<small style="color: olive;">という使用例もありました。
blue green red gray white oliveの６種あります。
gray white oliveは使用例も少ないのでcssに動かしていいのかな、と悩んでいます。

[AppVeyorBot]

❌ Build sakura 1.0.4241 failed (commit b57625633d by @dep5)

[AppVeyorBot]

❌ Build sakura 1.0.4248 failed (commit 6fbd672383 by @dep5)

[AppVeyorBot]

✅ Build sakura 1.0.4252 completed (commit 442c9eeeed by @dep5)

[berryzplus]

「客観的に妥当かどうか」を判断できる材料がそろっていないように見えます。

変更内容の説明から3stepで作業していると思いますが、1コミットにまとまってしまってるので妥当性の判断ができません。

1. strongをspanに正規表現で一括置換（人為的ミスの余地なし）
2. 一部spanの置換を手動でrevert（人為的ミスの余地あり）
3. font閉じタグの不具合を修正（人為的ミスの余地あり）

レビュー側で検証したらいい話ではあるんですが、めんどいです。

[AppVeyorBot]

✅ Build sakura 1.0.4262 completed (commit 4364fb5d81 by @dep5)

[dep5]

berryzplus さん
コメントありがとうございます。

2. 一部spanの置換を手動でrevert（人為的ミスの余地あり）
   についてですが
   <strong>でgrepなどが可能かと思います。
   手動revertしたのは次のページです。
   それ以外は1にあたるかと思います。

plugin.chm 概要
help\plugin\Text\overview.html

sakura.chm Grep
help\sakura\res\HLP000067.html

Grep置換
help\sakura\res\HLP000362.html

Windows 10 でのファイル拡張子関連付け
help\sakura\res\HLP000374.html

Windows 10 でのファイル拡張子関連付けのための準備作業
help\sakura\res\HLP000375.html

3. font閉じタグの不具合を修正（人為的ミスの余地あり）
   についてですが
   help/sakura/res/HLP000089.html
   ここでの混入となります

[dep5]

style属性だと
<span style="color: olive;">
のように空白が入ったりセミコロンが入ったり検索しづらいので
class属性に変更し、cssで指定します

あわせて
#800000をmaroonに
<span style="font-size: smaller;color: olive">を<small class="olive">に変更しています

blue green red gray white olive #800000(maroon)についての変換です

cssのスペースの使い方、行末のセミコロンについて、揃えました。

[berryzplus]

一旦閉じて仕切り直しませんか？

• 76d9f3b strongからspanへ変更
• d9ae4ff cssにspan.boldの設定

先行で出ていたコミットについて #1895 (review) で書いた検証を実施してみました。

確認結果は「妥当だと思います」です。
ただ、↓で指摘した過去の修正ミスは手当したほうがよい気がします。

追加分のコミット2件については、このPRと同時に実施してよい内容ではないように見えます。変更に対して「適切な理由」を見出せる修正なら含めてもいいのですが、思いつきませんでした。現状では「不適切な変更を含んでいるため」のrejectになります。

[berryzplus]

Suggested change

	<span class="bold">>Shift_JISのファイルでも、半角カタカナが含まれている場合、EUCと区別がつかないため、よく間違えます。<br>
	<span class="bold">Shift_JISのファイルでも、半角カタカナが含まれている場合、EUCと区別がつかないため、よく間違えます。<br>

過去の修正ミスにより > が2つ入っています。
同様に修正すべき箇所が他に2つあります。

[berryzplus]

• a3ff600 <span>のstyleをclassに変更
  「変更が必要な理由」が説明されていないように見えました。
• cf52197 cssの行末にセミコロンを追加
  「変更が必要な理由」が説明されていないように見えました。

何が問題で、どう対応するか？（どう対応したいか。）

詰め切らないうちにまとめてPRにしてしまった結果、
「なんかよくわからんやつ」になってるように見えます。

よくよく見るとPRのタイトルもおかしいですよね。
「ヘルプのタグを<strong>から<span>へ変更する」

strongをspanに変更したいのは何故か？
何か「目的」があって「変更という手段」を採るんじゃないんですかね？

何がしたいんでしたっけ？（分かってて書いてます。）

また、追加したコミットは目的と合致してますか？（合致しないと思って書いてます。）

[AppVeyorBot]

✅ Build sakura 1.0.4265 completed (commit f0d2d7c2f0 by @dep5)

[AppVeyorBot]

✅ Build sakura 1.0.4266 completed (commit cd1b79169b by @dep5)

[dep5]

対象ファイルが大量なのでいくつもPRをだすのは負担なのですが、どうにかならないですか？

1. cssのセミコロン
   このコミットの際にこう変更されていますが、セミコロンを忘れたままくっ付けています。

STRONG {
	COLOR: #909; BACKGROUND-COLOR: transparent
	BORDER-RIGHT: 3px; BORDER-TOP: 3px; BORDER-LEFT: 3px; BORDER-BOTTOM: 3px double
}

しかし、改行には宣言の区切りの働きはないので

STRONG {
	COLOR: #909;
	BACKGROUND-COLOR: transparentBORDER-RIGHT: 3px;
	BORDER-TOP: 3px;
	BORDER-LEFT: 3px;
	BORDER-BOTTOM: 3px double
}

こう解釈され、
BACKGROUND-COLOR: transparent
BORDER-RIGHT: 3px;
の二つが無効になっています

Edgeの開発者ツール

#1883 (comment)

これを修正するのが目的ですが、なぜこのようなミスが起きたかというと、
cssの行末のセミコロンがあったりなかったりなので無くてもよいだろう
と思われたのだと思うので、この際全部にセミコロンをつけてしまおうというコミットです。

2. <span>の色指定
   CSSを活用して<strong>と色付き文字を整理する #1895 (comment)
   変更理由はこれではだめですか？
   後でclass名を変える際も一括変換がしやすくなります

<strong>を<span>で太字に戻す
→<span>のstyleが気になる
で気になった部分をまとめたのですが

「#1505 で行われた変更について、再検討してみる（ついでにcssも見直す）」
だとどうでしょうか？そういうつもりでまとめてもよいかと思っていたのですが…

<b>→<strong>→<span>
<font>→<span style...>→<span class...>
以前 → #1505 → 今回の変更

このPRへのコミットはこれで終わりにします。
タイトルと概要を直せばよいならそうします。
複数に分けないと駄目な場合はどう分けるか指示してください。

[berryzplus]

追加の変更の意図は理解しました。

• セミコロンの問題について、
  「原因コミットはこれです」に対して
  修正対策を行わず「別な対応」をしている点が気になりました。
  不具合修正なのであればそうすべきだと思います。
  追加対応を行うかどうかは好みですが、個人的には不要と思います。

• spanの色指定の方法を変える件について、
  redやblueは、boldとは違うと思うので腹落ちしていません。
  「文字色をredにする」は実装的には「color: red;」ですが、
  製作者の意図は「強い注意喚起」だったり「エラー」だったりなはずです。
  だとするとredやblueの色名をスタイルにして一括置換できるようにするのは問題で、技術的負債を作る行為のような気がします。

「https://github.com/sakura-editor/sakura/pull/1505 で行われた変更について、再検討してみる（ついでにcssも見直す）」
だとどうでしょうか？そういうつもりでまとめてもよいかと思っていたのですが…

その意図であればそう書いてください。
「1505(ヘルプHTMLの修正)の見直し」なら理解できる内容と思います。

1903のコミット一覧では、何を不具合と修正していて、何を改善として修正しているかが分かると思います。

タイトルと概要を実態と合わせてもらえればあとの評価は実施しようと思います。

ところでこの指摘はスルーですかね？
#1505 の不具合修正が主題であれば直さないのはおかしいような。

[dep5]

#1895 (comment)
スルーしていません。966978d で対処しています。
不具合修正がメインというわけではありません。
警告対処の暫定対応と思われるので、別の方法があるのでは、という提案のつもりです。
#1505 の変更をすべて見たわけではありませんし・・・

「文字色をredにする」の使用例を見たところ、おっしゃるような「注意喚起」もありましたが、
大部分は色分けする程度の意図しか無いように思います。
色を指定しているのに、これは注意喚起だから何のクラスにして、これは同じ色だけど注意喚起じゃないから別のクラスにして、そして、CSSで別のタグに同じスタイルを指定したほうがいいということですか？

html中で直接スタイル指定は控えたほうがいいように思うので今回はCSSに移動するだけにしたいと思います。
必要ならば個別にタグかクラス名を変更、または新規追加しようと思いますが、どうでしょうか

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[AppVeyorBot]

✅ Build sakura 1.0.4270 completed (commit d0215ae6ad by @dep5)