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

レベル30到達予定日を見れる機能の追加 #10

Merged
merged 4 commits into from Jun 11, 2017

Conversation

Projects
None yet
2 participants
@mimikun
Contributor

mimikun commented Jun 8, 2017

レベル99到達予定日のところにレベル30到達予定日が無かったので機能追加しました。

@muziyoshiz

This comment has been minimized.

Owner

muziyoshiz commented Jun 8, 2017

機能提案ありがとうございます。
ただ、このpull requestは、そのままだと採用するのが難しいです。
少し長くなりますが、理由を解説します。

Admiral Statsのデザインは、Bootstrapというフレームワークを採用して作られています。
Bootstrapはグリッドシステムというものを採用していて、
これはページの横幅を12個のグリッドに区切り、各要素の長さをグリッドの数で表現します。

参考: http://websae.net/twitter-bootstrap-grid-system-21060224/

で、今回直してくれた「レベル 99 到達予想日」の表もグリッドで表現されているのですが、
今回列を足したことでグリッドの合計が14になっていて、
ブラウザによっては以下のようにレイアウトが乱れてしまっています。

20170608_pr10

なので、どれかの列を削ってグリッドの合計を12にするか、
艦娘一覧みたいに、一度に表示するグリッドの数を12個にする仕組みを入れる必要があります。
(艦娘一覧で、レベルと★を一度に表示していないのはそういう理由だったりします)

@mimikun

This comment has been minimized.

Contributor

mimikun commented Jun 11, 2017

レイアウトが崩れてしまう問題について、

艦娘一覧のページみたいに表示・非表示のボタンを追加したら上手くいきました。
レベル99非表示でレベル30が表示され、レベル30非表示でレベル99が表示されるといった感じです。

プルリクどう送ればいいですか?

@muziyoshiz

This comment has been minimized.

Owner

muziyoshiz commented Jun 11, 2017

修正ありがとうございます。確認します。
Pull request用に作成したリモートのブランチに、その修正内容をpushしてもらえば、
私の方でも見られるようになると思いますよ。

@muziyoshiz

This comment has been minimized.

Owner

muziyoshiz commented Jun 11, 2017

修正内容確認できました。以下の点だけ修正してもらったらacceptします。

  • Gemfile.lock を元に戻す(恐らくこの修正が原因で TravisCI のビルドを通っていない)
  • vagrant/playbook.retry の削除(あとで .gitignore に追加しておきます)

コメントで ? と書いてある部分は、マージ後に私の方で直します。

@mimikun

This comment has been minimized.

Contributor

mimikun commented Jun 11, 2017

今修正しました。多分直ってると思います…

@muziyoshiz muziyoshiz merged commit 74ec514 into muziyoshiz:master Jun 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@muziyoshiz

This comment has been minimized.

Owner

muziyoshiz commented Jun 11, 2017

ありがとうございます。マージしました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment