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

Imprv/gw5589 bot type ui #3603

Merged

Conversation

kaori-tokashiki
Copy link
Contributor

@kaori-tokashiki kaori-tokashiki commented Apr 8, 2021

タスク

  • GW-5589 Bot type選択のUI改善(defalt)
  • GW-5594 Bot type選択時のUIを整える
    PR分けてレビュー依頼出そうと思っていたのですが、同時にやってしまいました。
    完全に再現するとなると大変なので、微調整やコンポーネント化(共通化)は後続タスクで着手します。

表示

  • XD
  • なぜか中国語の表示が切り替えられなかったので日本語版と英語版のみにしてます。

日本語

Screen Shot 2021-04-08 at 21 33 23

ダークモード

Screen Shot 2021-04-08 at 21 34 01

後続タスク

  • GW-5607 bot type項目のコンポーネント化
  • GW-5608 bot type ui 微調整 タスク
    • margin / padding / 文字やアイコンの大きさ調整/ border radius
    • Official botのバッジとアイコン

Comment on lines 595 to 614
/*
Slack Integration
*/
.selecting-bot-type {
.bot-type-disc-easy {
color: #33d541;
}
.bot-type-disc-normal {
color: #e6a63c;
}
.bot-type-disc-difficult {
color: #ff5757;
}
.bot-type-disc-possible {
color: $info;
}
.bot-type-disc-impossible {
color: $gray-500;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここでのみ使用する色は直書きでカラーコード を書いています。

Comment on lines 191 to 201
.selecting-bot-type {
.btn-link {
font-size: 1rem;
cursor: pointer;
}
.supplementary-desc {
font-size: 1rem;
}
.badge-info {
font-size: 0.6rem;
}
Copy link
Contributor Author

@kaori-tokashiki kaori-tokashiki Apr 8, 2021

Choose a reason for hiding this comment

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

見出しの隣にあるアイコンやバッジは、headerにつられてサイズが大きくなってしまうのでfont-sizeを指定しています。

これは、ページの見出しにある編集アイコンを参考にしています。
Screen Shot 2021-04-08 at 17 03 17

@@ -84,6 +84,7 @@
}

.admin-bot-card {
min-width: 280px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

min-widthを指定することで、文字のはみ出しを防いでいます。

Copy link
Contributor

@itizawa itizawa left a comment

Choose a reason for hiding this comment

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

コメントしました。確認お願いします

@@ -103,6 +103,16 @@ const SlackIntegration = (props) => {
break;
}

const showBotTypeLebel = (level) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Level ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修正しました...!

<i className={`fa fa-external-link btn-link ${currentBotType === 'official-bot' && 'bg-primary text-light'}`} aria-hidden="true"></i>
</h3>
</div>
<div className="card-body p-4">
Copy link
Contributor

Choose a reason for hiding this comment

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

丸みは消滅しました...?
その結果微妙な白いスペースが... 😭

スクリーンショット 2021-04-09 12 18 51

Copy link
Contributor

Choose a reason for hiding this comment

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

https://zenn.dev/seya/articles/f642acf1c47358

三角は意外と簡単に実装できるかも?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます!
ただ、武井さんと話し合った結果、三角いらないだろうという話になりました。。。

<h5 className="card-title">Custom Bot (With Proxy)</h5>
<p className="card-text">This is a wider card with supporting text below as a natural lead-in to additional content.</p>
<div
className={`card admin-bot-card mx-3 rounded shadow ${currentBotType === 'custom-bot-with-proxy' && 'border-primary'}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

この書き方、僕も前やっていたのですが。 false が混入してしまうのでやめたほうが良いですね
スクリーンショット 2021-04-09 12 21 04

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なるほど、修正しました。

<span className="supplementary-desc mr-2">
{t('admin:slack_integration.selecting_bot_types.with_proxy')}
</span>
<i className={`fa fa-external-link btn-link ${currentBotType === 'custom-bot-with-proxy' && 'bg-primary text-light'}`} aria-hidden="true"></i>
Copy link
Contributor

Choose a reason for hiding this comment

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

これって Docs へのリンクですよね。
忘れてしまいそうなので、後ほど追加する予定であれば TODO コメントをしておいて欲しいですね

.selecting-bot-type {
.btn-link {
font-size: 1rem;
cursor: pointer;
Copy link
Contributor

Choose a reason for hiding this comment

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

btn-link はもともと cursor: pointer; が設定されていそうです

Copy link
Contributor Author

@kaori-tokashiki kaori-tokashiki Apr 9, 2021

Choose a reason for hiding this comment

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

指摘ありがとうございます。削除しました。

font-size: 0.6rem;
}
.border-primary {
border-width: 2px;
Copy link
Contributor

Choose a reason for hiding this comment

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

その結果微妙な白いスペースが... 😭
上記のスペースの原因はこれかも?

Comment on lines 99 to 103
.admin-bot-card {
min-width: 280px;
cursor: pointer;
border-radius: 0.5rem !important;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

!importantつけないと反映されないので追加しました。

Comment on lines +135 to +136
{/* TODO: add an appropriate link by GW-5614 */}
<i className="fa fa-external-link" aria-hidden="true"></i>
Copy link
Contributor Author

@kaori-tokashiki kaori-tokashiki Apr 9, 2021

Choose a reason for hiding this comment

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

linkの指定は後続タスクで着手します
TODO commentを記載しました。

  • GW-5614 Docsに飛ぶリンクを指定する

@kaori-tokashiki
Copy link
Contributor Author

kaori-tokashiki commented Apr 9, 2021

FB対応後お対応後の見た目です。
Screen Shot 2021-04-09 at 15 58 57

}
.admin-bot-card {
min-width: 280px;
cursor: pointer;
Copy link
Contributor

Choose a reason for hiding this comment

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

twbs/bootstrap#23709
role='button' 使ってねって書いてますね

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修正しました。

@kaori-tokashiki kaori-tokashiki merged commit 066f408 into feat/growi-bot Apr 9, 2021
@kaori-tokashiki kaori-tokashiki deleted the imprv/gw5589-design-for-selecting-bot-types branch April 9, 2021 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants