Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

[package] Chapter APIを導入し、elm-emakiをより楽に記述できるようにする #34

Merged
merged 6 commits into from
Mar 4, 2024

Conversation

pxfnc
Copy link
Collaborator

@pxfnc pxfnc commented Feb 11, 2024

なぜやるのか

既存のEmaki.Controlは、uiをコントロールするためのviewは提供していますが、コントロールするviewの状態と、コントロールする対象のviewのモデルの統合やデータの結合をユーザが記述しなければならず、利用者にとって負荷が高い状態になっています。

このPRで作成したChapter APIは、コントロールの集まりとコントロール対象のviewを組にしたChapterという概念を導入し、controlの状態管理のためのコードをユーザが自前実装をしなくて良くなります。

壮大な全体の計画は下記issue参照

このPRで実現したことサマリ

  • Emaki モジュールがチャプターを実行するためのrunChapterをexport
  • Emaki.Chapter が、viewとコントロールのリストを組みにした構造 Chapter データ型と、それを作るためのchapter関数をexport
  • Emaki.Chapter.Control chapterに渡すためのコントロールたちがたくさんあるモジュールにする予定。今はtext inputの雑実装のみある

詳細はコメントに記述しました!

このPRがmergeされた後にやる予定

  • ChapterのUIをexampleから移植
  • 既存のcontrolのchapter API準拠
  • examleのリライト

Copy link
Owner

@y047aka y047aka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main.elmがEmakiモジュールの新APIで期待通りに動きさえすれば、ドキュメントや細かい調整は別PRでも良いですよ!

@pxfnc
Copy link
Collaborator Author

pxfnc commented Feb 11, 2024

新しいAPIはexampleをほぼ全部rewriteすることになりそうで難しそう。小さいPRで徐々に移行するプランとして

  1. このPRの実装をEmaki.Chapterモジュールとしてexportして、既存の仕組みと独立した形でmergeしてしまう
  2. exampleにあるchapterに対応するUI部分をEmaki.Chapterに実装する
  3. いまのEmaki.ControlをChapter APIに適合させた Emaki.NewControlを作る
  4. あたらしいChapter APIのためのあたらしいexampleを作る
  5. Chapterの集まりを表示するためのAPI設計 + 開発
  6. 今まであったexampleをChapter APIでリライト
  7. 古いコードの削除

@pxfnc
Copy link
Collaborator Author

pxfnc commented Feb 11, 2024

4までは完遂させたい

@y047aka

This comment was marked as resolved.

package/src/Emaki.elm Outdated Show resolved Hide resolved
@pxfnc
Copy link
Collaborator Author

pxfnc commented Feb 11, 2024

📝 いい感じの実装ができたからこのブランチのコードを分割してreviewに回す

Comment on lines +1 to +5
module Emaki exposing (Model, Msg, runChapter)

import Browser
import Emaki.Chapter as Chapter exposing (Chapter)
import Html.Styled as Styled
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

今はrunChapterしかないけど、いつかrunChaptersみたいな感じのものができるはず

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

トップレベルのAPIについては、おそらくはBrowser.sandboxと対応させてsandboxにするのが良いなぁと思っています。実装するタイミングで適切なほうを選べば良いけど。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

入力となる型が { init: model, view : model -> viewLike, update : msg -> model -> model }な構造でしたら名前をsandboxにしても良いと思うのですが、emakiを使う人がやりたいのは組み立てたEmakiを実行できるProgramに変換することなので、runChapterもしくはbuildあたりが適切かなぁと思いました

逆にsandboxだとBrowser.sandboxと入力が全然違うのになんでsandboxという名前なのかで混乱しちゃいそうな気がしています

@@ -0,0 +1,68 @@
module Emaki.Chapter exposing (Chapter, Model, Msg, chapter)
Copy link
Collaborator Author

@pxfnc pxfnc Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emakiで閲覧したいview関数と、それの入力を操作するためのパーツを組み合わせたデータをChapterと呼ぶことにしました。ChapterもTEAっぽくなっています

Comment on lines +13 to +22
type alias Chapter props =
ChapterInternal.Chapter props


type alias Model props =
ChapterInternal.Model props


type alias Msg =
ChapterInternal.Msg
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

回りくどい実装ですが、Internalモジュールにしてelm.jsonでexposeしないようにするとライブラリ利用者がInternalモジュールを使って悪さできないようになります

中身の構造を利用者に見せないことで、emakiライブラリ開発者は好きに内部構造をリファクタできるようしつつ、利用者は中身の構造に誤って依存するようなことがなくなります

Comment on lines 7 to 12
type Control props
= Control
{ init : Value
, view : Value -> Html Value
, updateProp : Value -> props -> props
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全てのコントロールは、

  • コントロールの状態としてValueを持っていて
  • 状態からコントロールの表示する方法を知っていて
  • コントロールの状態が viewの引数をどう変化させるべきかを知っている

という定義にしました。

その結果、Controlは全て同じ型になるため、ChapterでList (Control msg)として持たせられるようになるため、利用者がコントロールを追加/削除する時にコードのあちこちを修正する必要はなくなります

Comment on lines 43 to 54
chapterView : Model props -> Styled.Html Msg
chapterView { viewState, controlsState } =
Styled.div []
[ Styled.fromUnstyled (view viewState)
, controlsState
|> JD.decodeValue (JD.list JD.value)
|> Result.withDefault controlInits
|> List.map2 (<|) controlViews
|> List.map (Styled.li [] << List.singleton)
|> List.indexedMap (\i -> Styled.map (ChapterInternal.UpdateControlAt i))
|> Styled.ul []
]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

雑にdivで作っちゃっていますが、コントロール対象のviewとコントロールをそれぞれ表示しているような実装です

Comment on lines +56 to +63
update (ChapterInternal.UpdateControlAt idx newValue) { viewState, controlsState } =
{ viewState = Maybe.withDefault (always identity) (Array.get idx controlOnChanges) newValue viewState
, controlsState =
controlsState
|> JD.decodeValue (JD.array JD.value)
|> Result.map (Array.set idx newValue >> JE.array identity)
|> Result.withDefault controlsState
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idx番目のコントロールが更新されたことがmsgからわかるので、jsonデコードして該当箇所を更新します。

この辺りはライブラリ利用者が直接Chapterの状態をいじると平気で壊れる危ない実装なのですが、ライブラリからexportしないことで利用者に触らせないようにします

@pxfnc
Copy link
Collaborator Author

pxfnc commented Feb 20, 2024

計画が壮大すぎたので、壮大な計画はissue側に移し、このPRでやったことについての説明をPRのdescriptionに記載します〜

@pxfnc pxfnc marked this pull request as ready for review February 20, 2024 13:54
@pxfnc pxfnc requested a review from y047aka February 20, 2024 13:55
@pxfnc pxfnc changed the title feat: Chapter APIを導入し、elm-emakiをより楽に記述できるようにする [package] Chapter APIを導入し、elm-emakiをより楽に記述できるようにする Feb 22, 2024
, controlsState
|> JD.decodeValue (JD.list JD.value)
|> Result.withDefault controlInits
|> List.map2 (<|) controlViews
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

かっこいいね😎

Comment on lines +7 to +8
import Html exposing (Html)
import Html.Styled as Styled
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

どっちが良いというものではないんだけど、現状はこのままが良いと思う反面、気持ちの面では将来的にas Htmlにしたくなるかもしれない点だけメモしておきます。

```elm
import Emaki exposing (runChapter)
import Emaki.Chapter exposing (chapter)
import Emaki.Chapter.Control as Control
Copy link
Owner

@y047aka y047aka Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

現時点ではこれでOK。
ただし、ChapterとControlの結びつきが少し強すぎるようにも見える。
ControlはChapterに対して作用するものではないので。
ここの関係性は今後の開発に応じて変化しそうだなと。


ちなみに、現時点では将来的に↓のような階層構造になると考えています。

Emakiアプリケーション
 > List Chapter
 > List Section
 > コンテンツ(Markdownテキストや、現playgroundに相当する小さいアプリケーションなど)
 > List Control

  • 小さいアプリケーションの層には他のアプリ(例えばキーノートなど)が入る可能性がある
  • 敢えてplaygroundのみに機能を限定する可能性はあるが、その場合にもEmaki.Chapter.Controlだと若干不自然?

(細かく見ていくと「Sectionの名前が他と被りやすいので採用しない」などもあり得ますが、いったん気にしない)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type alias Emaki =
  { chapters : List Chapters
  , sections : List Section
  , content : MarkdownText | PlaygroundApp
  , control : List Control
  }

ということでしょうか? あんまりピンとこなかったのでオフラインで会うときにしっかり認識合わせしたいです!

今回の自分の実装はEmaki.Chapter名前空間っぽく作ったので、最終的なモジュール構造が決まり次第これをバラして埋め込んでいければ良いかなと思っているので、このままmainに取り込ませていただきます 🚀

pxfnc and others added 2 commits March 4, 2024 23:40
Co-authored-by: Yoshitaka Totsuka <y047aka@users.noreply.github.com>
@pxfnc pxfnc merged commit d64dc5a into main Mar 4, 2024
1 check passed
@pxfnc pxfnc deleted the feature/chapter-api branch March 4, 2024 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants