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

【石川さん仕様確認待ち/確認中・対応待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 #2173

Draft
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

mtdkei
Copy link
Contributor

@mtdkei mtdkei commented Aug 15, 2024

チケットへのリンク / 変更の理由(元のissueがあればリンクを貼り付ければOK)

#1857 のタイトルで「そもそもコアのUI・挙動に合わせたい」にあったため。
「どういう変更をしたか?」で、@kurudrive さんの意図通りのプルリクになっているかを確認してください。(プルリクの挙動自体は確認後、修正の方向性を決めます。)

どういう変更をしたか?

Outerの レイアウト設定 > 余白 (左右) や余白 (上下)で設定済みの値をコアの余白システムに自動に割りあてる処理をしました。

この確認が終わったら従来の余白システムの設定を外したプルリクをあげます。

スクリーンショットまたは動画

変更前 Before

image

変更後 After

image

実装者の確認事項

実装者はレビュワーに回す前に以下の事を確認してチェックをつけてください。

  • 複数の意図の変更 ( 機能の不具合修正 + 別の機能追加など ) を含んでいないか?
  • Files changed (変更ファイル)の内容は目視で確認したか?
  • readme.txt に変更内容は書いたか?
  • 本当にちゃんと確認をしたか?

プログラムの変更の場合

テストを書かないのは普通ではありません。書けるテストは極力書くようにしてください。

  • 書けそうなテストは書いたか?

変更内容について何を確認したか、どういう方法で確認をしたかなど

  1. 以下のtxtにあるコードをブロックエディターに貼り付け。
    outer-padding-core.txt
  2. 余白(左右)と余白(上下)が以下のようになるか確認。

余白(左右)

  • 「コンテナエリアに合わせる」 : コンテンツ幅になるようにする( .is-layout-constrained containerを出力 )
  • 「アウターエリア内に余白を追加する」: paddingの左右に 4em を付与
  • 「アウターエリア内の余白を無くす」: paddingの左右に 0px を付与

余白(上下)

  • 「標準の余白を使用」: paddingの上下に 4em を付与
  • 「標準の余白を使用しない」: paddingの上下に 0px を付与
  1. フロントエンドで2の状態を維持できているかを確認。

レビュワーに回す前の確認事項

  • 実装者はこのテンプレートのチェック項目をちゃんと確認してチェックしたか?

レビュワー確認方法・確認内容など

  1. 以下のtxtにあるコードをブロックエディターに貼り付け。
    outer-padding-core.txt
  2. 余白(左右)また、余白(上下) に合わせて以下のようになるか確認。

余白(左右)

  • 「コンテナエリアに合わせる」 : コンテンツ幅になるようにする( is-layout-constrained containerを出力 )
  • 「アウターエリア内に余白を追加する」: paddingの左右に 4em を付与
  • 「アウターアイテムエリア内の余白を無くす」: paddingの左右に 0px を付与

余白(上下)

  • 「標準の余白を使用」: paddingの左右に 4em を付与
  • 「標準の余白を使用しない」: paddingの左右に 0px を付与
  1. フロントエンドで2の状態を維持できているかを確認。
  2. 既存のブロックで「ブロックのリカバリーを試行」が出ないことを確認。

また、複数の環境下で確認しました。


レビュワー向け

レビュワーが確認して変更が反映されていない場合の確認事項

レビューしてみて意図した動作をしない場合は再度ビルドするなど以下の項目を確認してください。

  • プルしたか?
  • ビルドしたか?
  • ビルドしたディレクトリは正しいか(別の開発環境のディレクトリを見ていないか)?
  • npm install したか?
  • composer install したか?
  • キャッシュをクリアして確認したか?

@mtdkei mtdkei changed the title [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 【確認待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 Aug 15, 2024
@mtdkei mtdkei marked this pull request as ready for review August 15, 2024 23:30
@drill-lancer
Copy link
Member

drill-lancer commented Aug 19, 2024

@mtdkei
「コンテナエリアに合わせる」 : コンテンツ幅になるようにする( .is-layout-constrained containerを出力 )
上記は確認できたのですが
それ以外はどこに反映されているのかがわかりません。
vk_outer-paddingVertical-use とかのクラスでしょうか?

@mtdkei
Copy link
Contributor Author

mtdkei commented Aug 19, 2024

@drill-lancer
ご確認ありがとうございます。詳しく書いてなくてすみません。

Outerで

  • 左右: アウターエリア内に余白を追加する
  • 上下: 標準の余白を使用しない

の設定をしたときはフロントエンドで

<div class="wp-block-vk-blocks-outer vkb-outer-9f74207a-e06b-4185-a5c3-51352f6aa401 vk_outer vk_outer-width-normal  vk_outer-bgPosition-normal" style="padding-left:4em;padding-right:4em;padding-top:0;padding-bottom:0"><span class="vk_outer-background-area has-background has-pale-pink-background-color has-background-dim" style="opacity:0.5"></span><div><div class="vk_outer_container">
<p>左右: アウターエリア内に余白を追加する</p>



<p>上下: 標準の余白を使用しない</p>
</div></div></div>

のように、wp-block-vk-blocks-outerのdivに
style="padding-left:4em;padding-right:4em;padding-top:0;padding-bottom:0
が設定されているかを確認していただければと思います。

@sysbird sysbird changed the title 【確認待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 【確認中】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 Aug 19, 2024
@sysbird
Copy link
Member

sysbird commented Aug 20, 2024

@mtdkei
確認しています
動作が不安定ですので、気付いたことから上げていきます
小出しになったらすみません

Outer(全幅)が表示されない

Issue #1857 の書き方が適当でもうしわけありません
私が困っていたのは Outer(全幅)の表示です
このブランチでは、Outer(全幅)が表示されなくなっているので確認が進んでおりません
このブロックのコードを添付します
outer.txt

develop

下記は Lightning の表示(正常)です、これが tt-4 ではコンテンツ幅が合ってないというのが #1857 です
develop

このブランチ

Lightning 全幅が左に寄ってるのと、コンテンツ幅が合ってない(合ってるけどずれているだけかも)、余白の出かたが異なる
current

リカバリーがでる場合がある

master(develop) で作成した Outer がこのブランチでリカバリーがでる場合があります
たまたま作成していたパターン 農家_トップページ ですが、
区切りギザギザのためかもしれません
一部ブロックのコードを添付しますね
nouen.txt

#1857 はグループブロックを使うことで回避できますので、優先度は低めです〜

@sysbird sysbird changed the title 【確認中】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 【確認中・対応待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 Aug 20, 2024
@mtdkei
Copy link
Contributor Author

mtdkei commented Aug 20, 2024

@sysbird
ご確認ありがとうございます!
全幅やリカバリーについて後ほど確認させていただきます。

Lightningでは「コンテンツ幅に合わせる」が効いているのは、コンテンツ幅に合わせる時に使用されるcontainer用のCSSが書かれているため問題がありませんでした。
デフォルトテーマではその様なCSSがないことが「コンテンツ幅に合わせる」が効かない部分かと思います。
また、デフォルトテーマではentry-bodyクラスのようなコンテンツを囲うdiv等のHTMLでコンテンツ幅を調整しおらず、個々のブロックごとにコンテンツ幅を調整するCSSが当たってるようなため、containerを効かせるのが難しいです。(こちらの勉強不足でなにかしらの方法があるのかもしれません…!)

また、issueについてこちらの理解が乏しくててすみませんが、issueの書き込み内容やタイトルが書き換わっている部分をみて、少なくともコンテンツ幅の処理については機能として無くす方向であるものと思ってました。こちらの認識に齟齬がある気がしてきましたので、お手数ですがご確認いただけたら幸いです。
issue内で書き込みをされた @kurudrive さんにもこの点をご確認いただけたらと思います。
確認後、改めて修正、場合によっては別のissueを立てます。

@mtdkei mtdkei changed the title 【確認中・対応待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 【石川さん仕様確認待ち/確認中・対応待ち】 [ Outer ] 従来の余白システムの値をコアの余白システムに自動に割りあてる処理追加 Aug 20, 2024
@mtdkei mtdkei marked this pull request as draft August 22, 2024 05:51
@sysbird
Copy link
Member

sysbird commented Aug 23, 2024

x-t9 や Lightning のなかで Outer の css を書いているところ、
どうにか VK Blocks で吸収できるとスマートですが、ここまで運用しているブロックですので途中で変えるの難しいですね

コアのブロックでコンテンツ幅は、下記のスタイルが当たってるようです
max-width: var(--wp--style--global--content-size);

VK Blocks のなかでスライダーはどうやってるのか見てみました
スライダー(全幅)内のスライダーアイテムには、コア(Gutenberg?)によって下記のスタイルが当たっているようです

.is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
  max-width: var(--wp--style--global--content-size);
  margin-left: auto !important;
  margin-right: auto !important;
}

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