Skip to content

Conversation

@maeda-m
Copy link
Member

@maeda-m maeda-m commented Aug 24, 2015

  • 全ての情報を JSON でレイアウトファイルに記録する
  • List 以外の図形の JSON スキーマを統一された理解しやすい構造へ改善
  • List の JSON スキーマを各セクション毎に各図形スキーマを含む構造へ改善
  • SVG の構造や属性の内包、SVG との依存を完全に排除する
    • 下位互換性を保つため古いスキーマの読み込み時は新しいスキーマに変換した後、読み込む
      • 旧スキーマ → 旧スキーマから新スキーマに変換(単独クラス) → JSONから生成 → 描画完了
      • 新スキーマ → JSONから生成 → 描画完了
    • スキーマフォーマットを pretty 固定

Refer to: thinreports/thinreports#4

@maeda-m maeda-m added the question Further information is requested label Aug 24, 2015
@maeda-m maeda-m self-assigned this Aug 24, 2015
@maeda-m maeda-m added this to the 1.0.0 milestone Aug 24, 2015
@hidakatsuya hidakatsuya modified the milestones: 0.9.0, 1.0.0 May 7, 2016
@hidakatsuya hidakatsuya added the enhancement New feature or request label May 7, 2016
@maeda-m maeda-m force-pushed the new-layout-schema branch 2 times, most recently from 284625c to 804ce4e Compare May 16, 2016 13:34
@hidakatsuya
Copy link
Member

hidakatsuya commented May 18, 2016

https://github.com/thinreports/thinreports-generator/blob/master/examples/list_events/list_events_0_8.tlf
page-footer, footertranslate が変換できてない。

--- a/examples/list_events/list_events.tlf
+++ b/examples/list_events/list_events.tlf
@@ -132,7 +132,7 @@
         "height": 50,
         "translate": {
           "x": 0,
-          "y": -111.9
+          "y": -161.9
         },
         "items": [
           {
@@ -219,7 +219,7 @@
         "height": 50,
         "translate": {
           "x": 0,
-          "y": -118.5
+          "y": -218.5
         },
         "items": [
           {
@@ -358,4 +358,4 @@
       20
     ]
   }

変更後の値は元の y 座標。

@hidakatsuya
Copy link
Member

https://github.com/thinreports/thinreports-generator/blob/master/examples/tblock_styles/tblock_styles.tlf
text-blockvertical-align が正しく変換できていない。すべて top になっている。

@hidakatsuya
Copy link
Member

https://github.com/thinreports/thinreports-generator/blob/master/examples/tblock_styles/tblock_styles.tlf

screen shot 2016-05-18 at 22 48 26
screen shot 2016-05-18 at 22 48 35

  • textline-height がおかしい。 line-height-ratio: 2, font-size: 18 なのに line-height: 39.xxx になってる
  • tblockline-height が変換できてない?空になってる

@hidakatsuya
Copy link
Member

@maeda-m

[Q] page-numbertarget って、"report" or "<list-id>" を返す? "" は返さない?

@hidakatsuya
Copy link
Member

[Q] page-numbertarget って、"report" or "<list-id>" を返す? "" は返さない?

"report" を返すのはやめよう。 list の id が "report" になってると変な動きになってしまうので。Schema の方も修正しておくけど、

  • target.empty? => for Report
  • target.present? => for List

としましょう。

@hidakatsuya
Copy link
Member

Slack:
Editorで一因を発見。"letter-spacing": nullとなっている。

これは修正済み?

goog.object.extend(hash, {
'reference-id': this.getRefId(),
'value': this.getDefaultValueOfLink(),
'mulitple-line': this.isMultiMode(),
Copy link
Member

Choose a reason for hiding this comment

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

typo multiple

@hidakatsuya
Copy link
Member

[Q] word-wrap の返す値は "break-word", "none" のみ? "" は返さない?

@maeda-m
Copy link
Member Author

maeda-m commented May 21, 2016

[Q] word-wrap: "" は返さない。デフォルト値は言語設定によって変化する。

@maeda-m
Copy link
Member Author

maeda-m commented May 21, 2016

[Q] page-number: 同意見。"report" というリストがあったらおかしなことになる。

@maeda-m
Copy link
Member Author

maeda-m commented May 21, 2016

TODO:

  • page-footer, footer の translate が変換できてない。
    • detailの高さを引かない値を持つ
  • "letter-spacing": null

DONE:

  • text-block の vertical-align
  • text-block の line-height
  • page-number の target
  • word-wrap の返す値

@maeda-m
Copy link
Member Author

maeda-m commented May 22, 2016

page-footer, footer の translate が変換できてない。

detail の高さ分を translate.y から引けば値が正しくなる。

@maeda-m maeda-m force-pushed the new-layout-schema branch from 95832fd to 699ccee Compare May 24, 2016 22:32
thin.core.AbstractTextGroup.prototype.setKerning = function(spacing) {
var layout = this.getLayout();
var element = this.getElement();

Copy link
Member

Choose a reason for hiding this comment

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

spacing は何が入る可能性があるの?コードでそれを表現しておいたほうが、あとでわかりやすいと思う。

Copy link
Member Author

Choose a reason for hiding this comment

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

#38 の負債。0.6 から継続して使用している 0.8 のレイアウトファイルは kerning が NaN になってしまっている。
0.9 で一掃する。1.0 では消すコードとなるので、メソッド化して deprecated とする。

/**
 * @deprecated See: https://github.com/thinreports/thinreports-editor/issues/38
 * @param {string|number} spacing
 * @return {string}
 */
thin.core.AbstractTextGroup.prototype.convertKerningToDefaultInSince06 = function(spacing) {
  if (isNaN(Number(spacing))) {
    spacing = thin.core.TextStyle.DEFAULT_KERNING;
  }

  return spacing;
};


/**
 * @param {string} spacing
 */
thin.core.AbstractTextGroup.prototype.setKerning = function(spacing) {
  var layout = this.getLayout();
  var element = this.getElement();

  spacing = this.convertKerningToDefaultInSince06(spacing);

* @return {goog.math.Coordinate}
*/
thin.core.ShapeStructure.BLANK_ = '';
thin.core.ShapeStructure.getTransLateCoordinate = function(transformElement) {
Copy link
Member

Choose a reason for hiding this comment

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

[いつか直したい] getTranslate なんだよなぁ

Copy link
Member Author

Choose a reason for hiding this comment

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

deprecated なメソッドなので 1.0 では消えます。

@hidakatsuya
Copy link
Member

[Q] 古いレイアウトを開くところの流れがわからないんだけど、

  • 実装は新しいレイアウトフォーマット(json)で開けるように修正した
  • 古いフォーマットの場合は、一旦新しいレイアウトフォーマットへ変換して読み込む

であってる?

pageMarginLeft.setValue(thin.layout.FormatPage.DEFAULT_SETTINGS['margin-left']);
pageMarginRight.setValue(thin.layout.FormatPage.DEFAULT_SETTINGS['margin-right']);
var defaultMargin = thin.layout.FormatPage.DEFAULT_SETTINGS['margin'];
pageMarginTop.setValue(defaultMargin[0]);
Copy link
Member

Choose a reason for hiding this comment

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

順番の仕様が曖昧なので、以下のようにしよう。 generator の方も修正する。

[top, right, bottom, left]

https://docs.google.com/spreadsheets/d/1iLqM1rdavuLLlbl_8xKODIIWZbHAE3D0A4AzhmK9_Pw/edit#gid=1893696187

Copy link
Member Author

Choose a reason for hiding this comment

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

あい。Editorは大丈夫だと思います。

Copy link
Member

Choose a reason for hiding this comment

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

ん?修正は必要だよね?

Copy link
Member Author

Choose a reason for hiding this comment

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

再設定したらおかしくなることが判明。

Copy link
Member

Choose a reason for hiding this comment

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

再設定したらおかしくなることが判明。

そもそも順番が違うと思う。今のコードを見る限り、 [top, bottom, left, right] になってる気がする。

Copy link
Member Author

Choose a reason for hiding this comment

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

ですな。勘違いしていた。

@maeda-m
Copy link
Member Author

maeda-m commented May 29, 2016

[Q] 古いレイアウトを開くところの流れがわからないんだけど、

実装は新しいレイアウトフォーマット(json)で開けるように修正した
古いフォーマットの場合は、一旦新しいレイアウトフォーマットへ変換して読み込む

古いフォーマットの場合を詳しく説明すると、次の流れになる。

  1. 旧レイアウトをメモリ上に読み込み、メモリ上にSVGおよびオブジェクトを展開する
  2. 1.で展開したメモリ上のオブジェクトから新しいレイアウトフォーマットへ変換する
  3. 2.で変換したフォーマットを読み込み、描画する

@hidakatsuya hidakatsuya changed the title Schema of new layout file New format for layout file May 29, 2016
@hidakatsuya hidakatsuya modified the milestone: 0.9.0 May 29, 2016
@hidakatsuya
Copy link
Member

古いフォーマットの場合を詳しく説明すると、次の流れになる。

なるほど。それぞれのコンテキストでクラスを分けれると良かったかもね(詳しく見てないので、そうなってるかもしれんが)まあ、変換のところは寿命が短いし、可能なら改善するぐらいでいいので、今回どうこうって話じゃないです。

@maeda-m
Copy link
Member Author

maeda-m commented May 29, 2016

Release 0.9.0 #59 と衝突する内容があるので、一旦、過去のコミットを改変します。

@maeda-m maeda-m force-pushed the new-layout-schema branch from 33e0091 to e6309d6 Compare May 29, 2016 13:44
@hidakatsuya
Copy link
Member

Release 0.9.0 #59 と衝突する内容があるので、一旦、過去のコミットを改変します。

あー、 layout/layout.js か。0.7 のサポート廃止はこの PR とは関係ないはずなので、この PR の変更からは外したほうがいいね。ちなみに、そういったちょっと関連性のない変更をやっちゃう場合でも、コミットさえ分けておけば、 cherry-pick で branch を自由に移動できるので便利。

@maeda-m
Copy link
Member Author

maeda-m commented May 29, 2016

コミットさえ分けておけば、 cherry-pick で branch を自由に移動できるので便利。

ほほう。調べてみる。

@maeda-m maeda-m force-pushed the new-layout-schema branch from e6309d6 to 32df8e9 Compare May 29, 2016 14:07
@maeda-m maeda-m force-pushed the new-layout-schema branch from 32df8e9 to 1965632 Compare May 29, 2016 14:44
@maeda-m
Copy link
Member Author

maeda-m commented May 29, 2016

Release 0.9.0 #59 と衝突する内容があるので、一旦、過去のコミットを改変します。

layout/layout.js の変更を取り消した。

var content = this.getFile().getContent();
if (/^data:(.+?);base64,(.+)/.test(content)) {
goog.object.set(object, 'data', {
'mime-type': RegExp.$1,
Copy link
Member Author

Choose a reason for hiding this comment

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

rake dev:checkimageshape.js:468: WARNING - References to the global RegExp object prevents optimization of regular expressions. と出ていたのを無視してビルドしたが、該当箇所が最適化されており、動作しない。

Copy link
Member

Choose a reason for hiding this comment

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

match でやれば良いのでは?

Copy link
Member Author

Choose a reason for hiding this comment

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

そうです。チャレンジなうです。

Copy link
Member Author

Choose a reason for hiding this comment

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

content が undefined であることが原因みたいです。

@maeda-m maeda-m merged commit 17ea011 into master May 31, 2016
@maeda-m maeda-m deleted the new-layout-schema branch May 31, 2016 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants