Skip to content
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

スリム幅を設定した記事・固定ページのページヘッダー部分がスリム幅にならない #1313

Closed
kutsu123 opened this issue Feb 16, 2023 · 22 comments

Comments

@kutsu123
Copy link

@inc2734
お疲れ様です。
Snow Monkey v19.1.3のスリム幅の指定の仕方の変更の影響か、ページヘッダー(タイトルやmeta情報などの部分)やアイキャッチがスリム幅にならず、従来のコンテンツ幅で表示されていましたので、お知らせします。

例:
https://demo.rui-jin-en.com/r002-lp-a/archives/39
※類人猿側でもCSSカスタマイズしててなんかわかりにくい例ですみません!!

@inc2734
Copy link
Owner

inc2734 commented Feb 16, 2023

あ、ほんとだ…
下記で div.u-slim-width を消したのですが、コンテンツについては CSS で幅設定をしたのに、記事ヘッダーは CSS での幅設定を追加してなかったみたいです。

3bb00ec#diff-4d9a2ec8bad992c50d47caedfaf8654847bbda3e55defff46206acea9f57dff1L26-L38

3bb00ec#diff-fe8afbfd7390ac64e5c3a92bd3d5f699fdba3ba4ccacb8302fac3d923c383f53L83-L104

ちょっとどう対応すれば良いか考えます!

@inc2734
Copy link
Owner

inc2734 commented Feb 16, 2023

スクリーンショット 2023-02-16 14 22 30

こうですよね?

@inc2734
Copy link
Owner

inc2734 commented Feb 16, 2023

これで良いかも…。

[data-has-sidebar="false"][data-is-slim-width="true"] .l-contents__container {
	--_container-max-width: var(--wp--custom--slim-width);
}

@kutsu123
Copy link
Author

@inc2734
ご返信・ご確認などありがとうございます
いただいた画像のスクショの見た目で当たってます(前のバージョンだとその見た目でした)

これで良いかも…。

[data-has-sidebar="false"][data-is-slim-width="true"] .l-contents__container {
	--_container-max-width: var(--wp--custom--slim-width);
}

確かに、l-contents__container にスリム幅のwidth値設定でも良さそうですね

唯一の懸念点というかこれまでとの差で言えば、.l-contents__container div.u-slim-width 開始するまでの間にパンくず(表示位置を「コンテンツの上」に設定した場合)が入る可能性があって、そちらの幅がコンテンツ幅だったのがスリム幅になりそうだなってことでしょうか。

ここはスリム幅になるほうが、見た目が揃って良いと思うので、私としてはありがたい点でもあります・

@inc2734
Copy link
Owner

inc2734 commented Feb 16, 2023

@kutsu123 ぐわーそうですね、確かに。。。パンくずの位置はコンテンツ幅に揃えるのとコンテナー幅に揃えるのがあって、それは .u-slim-width の中か外かで出し分けられていたので、現状だとどちらを選んでも同じ位置に表示されちゃいますね…。あまり HTML 構造を変えすぎるのも良くないと思うので、なるべく小さな変更で対応できないか検討してみます。

@inc2734
Copy link
Owner

inc2734 commented Feb 16, 2023

@kutsu123 master に変更を入れてみました。もしよかったら、pull & build して試してみていただけないでしょうか?

@kutsu123
Copy link
Author

@inc2734
度々のご返答・ご対応などありがとうございます。
確認したところ下記のような状態でした

  • パンくずはコンテンツ幅・コンテナー幅両方想定どおりに動きました
  • 投稿タイトル・meta情報ともにスリムになってました
  • アイキャッチ(コンテンツの上表示設定の場合)の幅はコンテンツ幅のままでした

アイキャッチにも max-width: var(--wp--style--global--content-size) の指定を入れる必要があるかもしれません

スクリーンショット 2023-02-17 8 51 16

@kutsu123
Copy link
Author

アイキャッチ箇所、類人猿のデモサイトではもともとスリム幅でもはみ出るぐらいの幅指定にカスタマイズしてて、そのことに慣れてしまってて気づかずですみません(お手数おかけします・・・)

@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

@kutsu123 アイキャッチ!確かにそこは制限されなくなっちゃいますね…。ご指摘ありがとうございます!

アイキャッチに max-width: var(--wp--style--global--content-size) でも良いかなと思ったのですが、よくよく考えるとアイキャッチ以外にもウィジェットエリアとかシェアボタンとかもあって、もっと言えばアクションフックで追加されるコンテンツとかもあるでしょうし…と考えると、個別に max-width を設定するのは応急処置的な感じになってしまうのかなぁと思いました。

まだどうするのが良いのかは考え中なのですが、スリム幅テンプレートのときは、

<div class="c-entry__body">
  <div class="u-slim-width">
    シェアボタン
    アドセンス
    アイキャッチ
  </div>

  <div class="c-entry__content>
    ...
  </div>

  <div class="u-slim-width">
    ウィジェットエリア
    シェアボタン
  </div>
</div>

みたいに .u-slim-width で囲うか、.u-slim-width じゃなくて .c-entry__content で囲う(スリム幅のときは中の要素の幅が勝手に狭まる)とか。が良いのかなぁと考え中です。

@kutsu123
Copy link
Author

あ、そうか!確かにウィジェットとかシェアボタンとか他にも色々な要素ありますよね。(そこまで考え及ばなかったしまった)
たぶん現状だと.u-slim-widthでくくるほうが現在の構造から破綻しにくいのかなと思いますが、ユーティリティクラスは使わないで済むなら使わないほうがいいのかな〜とか、全部.c-entry__contentに入れたほうがとてもシンプルで良さそうって思ったりもしてます(感想コメですみません)

@Olein-jp
Copy link
Contributor

全然ソースは読めていませんが、 @kutsu123 さんが言われているように、ユーティリティクラスは最後の砦的な感じが FLOCCS 感あると思うので、 c-entry__content に入れる方が良さげなのかなと思いました。

という感想でした。

@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

ちょっとやってみていたのですが、

.u-slim-width を使う

現状、template-parts/content/entry/entry.php は今のページがスリム幅かどうかの判別ができない。テンプレート引数を追加して、親テンプレートからバケツリレーでスリム幅かどうかの情報を伝える必要がある。フックなどで HTML を追加している場合は影響が出る可能性がある。

.c-entry__content を使う

現場、シェアボタンやウィジェッテリア等々は .c-entry__content のテンプレートパーツ(template-aprts/content/entry/content/*)の中で呼び出されていないので、呼び出し場所を変更するために各テンプレートパーツの変更が必要。フックなどで HTML を追加している場合は影響が出る可能性がある。

CSS でやる

.l-contents.c-entry の中も全て .c-entry__content 内のように子要素に margin-inline: auto; max-width: xxx で幅指定中央寄せする方法。フックなどで HTML を追加している場合は影響が出る可能性がある。

という感じでどれも一長一短でした。僕も .c-entry__content を使う方法か、あるいは CSS で頑張るかのどちらかが良いかなと思っていまして、ちょっと今 CSS で頑張るほうは試してみています。一段落したら push するのでお手すきのときにでも試していただけると大変うれしいです!!

inc2734 added a commit that referenced this issue Feb 17, 2023
@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

@kutsu123 @Olein-jp push しました!CSS 自体は少しシンプルになったと思います。が、やっぱりフックでコンテンツが追加されていたり、CSS で .l-contents.c-entry に関連する部分をカスタマイズしている場合が気になりますね…。

@kutsu123
Copy link
Author

kutsu123 commented Feb 17, 2023

@inc2734
度々のご対応・あと進捗の状況とかお知らせありがとうございます!

ちょっと手持ちのテスト環境で試してみました。フック周りは試してないですが、ウィジェットやもともと該当箇所に表示できる要素については全部スリム幅になってました

screencapture-maruttolp-local-archives-39-2023-02-17-15_19_35

@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

@kutsu123 ご確認ありがとうございます!

フック周りは試してないですが、ウィジェットやもともと該当箇所に表示できる要素については全部スリム幅になってました

フックで追加するだけなら正しい表示になりますが、それを CSS で margin:0 とかしてると左に寄っちゃいます。かといって Snow Monkey 側で margin:auto !important するのも微妙なので悩ましいところですが…。一応 Slack と Twitter にもこの issue を流したので、特に何も報告がないようであれば明日アップデートしちゃおうと思います。

@kutsu123
Copy link
Author

@inc2734
スリム幅と関係ないのですが、最新コミットのものだと、類人猿のパターンに影響があったため報告しておきます
(これは不具合というより、新しい仕様に追いついてない状態ゆえかもしれません、すみません)

セクション(全幅)のコンテナ(全幅に設定)のmax-widthが余白サイズ加味された値が入ってしまって、全幅になっていませんでした
スクリーンショット 2023-02-17 15 35 36

現行リリース済みのSnow Monkeyだと全幅のコンテナは100%で表示されてます
https://demo.rui-jin-en.com/r001-corp-a/

@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

うが、早速ですね…すみません…。下記の CSS が影響しているようです。

https://github.com/inc2734/snow-monkey/blob/master/src/css/app/foundation/_wordpress/_align/_app.scss#L56-L58

https://github.com/inc2734/snow-monkey/blob/master/src/css/app/foundation/_wordpress/_align/_app.scss#L68-L70

.alignfull.c-container {
  max-width: calc(100% - var(--_container-margin-right) - var(--_container-margin-left)) !important;
}

.alignfull.c-container {
  --wp--style--global--wide-size: min(100% - var(--_container-margin-right) - var(--_container-margin-left), var(--wp--custom--content-wide-width));
}

ざっと確認したところ消しても問題なさそうかなという感じですか、もしかしたらニッチなところで影響があって入れたかもなので、ちょっと夜時間があるときにでも見てみます。ご報告ありがとうございます!!

inc2734 added a commit to sass-basis/basis that referenced this issue Feb 17, 2023
@inc2734
Copy link
Owner

inc2734 commented Feb 17, 2023

先の CSS、なんで追加したのかはわかりました。セクション系ブロックの「コンテナーの配置」を全幅・全幅にしたときに、中身のコンテンツによってはコンテンツが画面右端に突き抜けてしまう問題があったからでした。

内容としては下記のトピックとほぼ同じです。
https://snow-monkey.2inc.org/forums/topic/iphone%e3%81%a7%e7%94%bb%e5%83%8f%e3%82%84%e3%82%b3%e3%83%b3%e3%83%86%e3%83%b3%e3%83%84%e3%81%8c%e7%94%bb%e9%9d%a2%e3%81%8b%e3%82%89%e3%81%af%e3%81%bf%e5%87%ba%e3%81%99/

で、この問題、.c-containermin-width: 0 にすることで解決できることがわかりました。
https://kudakurage.hatenadiary.com/entry/2016/04/01/232722

これをすることで、max-width: { min(100% - var(--_container-margin-right) - var(--_container-margin-left), var(--wp--custom--content-wide-width)); } みたいな複雑な計算式にしなくても、max-width: { var(--wp--custom--content-wide-width); } だけで良くなります。

そうすると、theme.json で指定しているコンテンツ幅(settings.layout.contentSize)も同様の複雑な計算式になっていたので、ここもシンプルなものに変えました。

@kutsu123
Copy link
Author

@inc2734
確認おそくなってすみません
最新コミットで無事全幅になってるの確認しました。 ご対応やあとどういうふうに対応したかまで教えていただきありがとうございます(自分もよくflexbox使うので、めっちゃ知見共有ありがたいです)

@inc2734
Copy link
Owner

inc2734 commented Feb 18, 2023

@kutsu123 というか土曜日にすみません!!一応 Snow Monkey 公式サイトには反映させてみましたが大丈夫っぽいです。午前中にアップデートしようとおもったのですが、Blocks のバク報告があり、それをやっていて時間切れになってしまったので月曜のアップデートになるかなと思います。
ご協力ありがとうございます!!

@kutsu123
Copy link
Author

@inc2734
さっそくのご確認ありがとうございます(平日あまり作業できてなくて、もともと今日仕事ちょっとする予定だったので無問題です) アプデの時期についても承知しました。お知らせありがとうございます。

すみません追加で1点気になる箇所見つけたので報告します。
昨晩の対応(22:17 5e0dcf4)で、これまで編集画面にあった左右の余白がなくなってました。

  • テンプレート:1カラムなど(フル幅は既存バージョンも余白なしなので対象外)

フロント側と揃えてる状況なので、そもそも余白がない(実際のサイズそのものである)のが仕様として正しいと思いますが、左右ちょっぴりでも余白あるほうがブロックの選択やテキストの入力がしやすいって思ってるので、もし検討の余地あったら編集画面だけ余白いれるのご検討いただけるとありがたいです。

▼既存バージョンの場合
スクリーンショット 2023-02-18 10 50 05

▼最新コミットの場合
スクリーンショット 2023-02-18 10 49 10

@inc2734
Copy link
Owner

inc2734 commented Feb 18, 2023

あー!や、不具合です、すみません。編集画面は確認してませんでした…。確認します!

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

No branches or pull requests

3 participants