Skip to content

add column settings into the hashtag timelines#96

Merged
takayamaki merged 2 commits intoimas:imastodonfrom
fvh-P:TagTL-setting
Dec 6, 2017
Merged

add column settings into the hashtag timelines#96
takayamaki merged 2 commits intoimas:imastodonfrom
fvh-P:TagTL-setting

Conversation

@fvh-P
Copy link
Copy Markdown
Collaborator

@fvh-P fvh-P commented Sep 23, 2017 •
edited
Loading

2017/12/03 Modified

Add column settings into the hashtag timelines. It enables to set the following settings for each hashtag columns.

カラム設定をハッシュタグタイムラインに追加します。これによってそれぞれのハッシュタグカラムごとに以下の設定が可能になります。

  • お気に入りタグ登録/解除ボタン (2017/12/03追加)
    すでにお気に入りに追加されていれば解除ボタンを、そうでなければ2つの追加ボタン(公開/未収載)を表示します。
  • ローカルのみ表示 (true / false)
    trueならサーバ内部のトゥートのみ表示します。(タグストリームをlocalに切り替えます)
  • Show replies (true / false) (2017/12/03に削除しました。タグTLにunlistedなリプライが流れないように #107 で解決されたためです。)
    falseならリプライを表示しません。 (タグTL上にin_reply_to入りのreply投稿も表示されてしまう #92)
  • Regex filter (string)
    正規表現にマッチしたトゥートを非表示にします。

settingsのinitialStateにtag: ImmutableMap()をつくり、その内部にtagIdをキーとして

ImmutableMap({
  shows: ImmutableMap({
    local: false,
  }),

  regex: ImmutableMap({
    body: '',
  }),
});

という構造を動的に格納しています。
SettingToggleやSettingTextにsettingsを渡すときに、settings.get(`${tagId}`)がundefinedであった場合、上記の構造を初期値としてSettingToggleやSettingTextに渡します。

確認済み

  • 各設定トグルのスイッチ動作やテキストボックスの入力動作、正規表現が正しく動作すること。
  • リロードしたときに各カラムの設定が復元されること
  • ストリームの切り替え
    自環境で確認する術がないため確認可能な方に確認していただきたいです。
    (2017/12/03)Chromeのデバッガーで、WebSocketの接続先が適切に切り替わっていることを確認しました。

要確認

  • リプライ表示/非表示の切り替え
    　ホームカラムのものをそのまま持ってきましたが動くかどうか不安です。
    　(自環境ではそもそもホームでもこの設定が動いていないように見える)
    2017/12/03解決。(機能削除のため)

また私のReact・Reduxについての知識は非常にあいまいで、変なコードを書いている可能性があるのでどんどんReviewをお願いします。

@fvh-P fvh-P force-pushed the TagTL-setting branch 2 times, most recently from a07832f to 33da05e Compare October 21, 2017 16:57
takayamaki
takayamaki previously approved these changes Nov 8, 2017
View reviewed changes
@takayamaki takayamaki self-requested a review November 8, 2017 18:29
@takayamaki takayamaki dismissed their stale review November 8, 2017 18:30

間違えて押しました

@fvh-P fvh-P added help wanted Solutions or ideas are needed priority low Don't care it is postponed labels Nov 25, 2017
@fvh-P fvh-P changed the title [Help wanted] add column settings into the hashtag timelines add column settings into the hashtag timelines Nov 25, 2017
@fvh-P fvh-P force-pushed the TagTL-setting branch 2 times, most recently from fc28f46 to ca412ba Compare December 3, 2017 11:08
@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 3, 2017

#107 でリプライ表示に関するものは解決されたと思うので、Show repliesトグルは削除しました。

また、お気に入りタグ登録/解除ボタンをカラム設定エリアのトップに表示する機能を追加しました。
既に登録済みのタグの場合は解除ボタンを1つ、未登録のタグの場合は登録ボタンを2つ(公開/未収載)表示します。非公開での登録ボタンは使用頻度が少ないと思われるうえに、設定エリアの縦幅が長くなっていってしまうので表示しません。（要望があれば対応します。）
fav3

@fvh-P fvh-P added enhancement New feature or request and removed help wanted Solutions or ideas are needed priority low Don't care it is postponed labels Dec 3, 2017
@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 3, 2017

また、#62 においても示されている通り、現在お気に入りタグの登録名は大文字小文字が区別されているため、大文字小文字含めて完全に一致するタグが登録されているときのみ解除ボタンが表示されます。

お気に入りタグについてこの仕様をどうするかはまた検討が必要かと思います。個人的には、大文字小文字が違うだけで、同じタグであっても別の公開範囲で登録しておけるのでこのままでもいいかなとは思いますが、お気に入りタグ一覧からタグカラムへ飛ぶとカラムidが小文字に直されてしまうため、大文字が入っているタグをクリックしてタグカラムに飛んでも、すべて小文字になっている同じタグが登録されていない限り「解除ボタン」ではなく「登録ボタン」が表示されるという動作になってしまうので、どうしたものかという感じです。

@takayamaki
Copy link
Copy Markdown
Member

takayamaki commented Dec 4, 2017

お気に入りタグ一覧からタグカラムへ飛ぶとカラムidが小文字に直されてしまう

についてはその元となる #62 が本家 tootsuite#4804 にて修正済みであるため、 #64 は戻してしまって問題ないと思います。

が、それはそうとReactのエラーが出ていて動作の確認自体が出来ておらず、調べたところnpmの
react-immutable-proptypes パッケージではnumber自体存在しないようなのですがそのあたりご確認いただけますでしょうか。
image

@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 5, 2017

column_settings.jsの23行目
tagId: ImmutablePropTypes.number.isRequired, ではなく、正しくは
tagId: PropTypes.number.isRequired, でした。

@takayamaki
Copy link
Copy Markdown
Member

takayamaki commented Dec 5, 2017 •
edited
Loading

お気に入りタグの登録/解除については動作を確認しました。

ColumnSettingsコンポーネント、SettingToggleコンポーネント、SettingTextコンポーネントについて、propのtagIDにnumberではなくstringが渡されているというwarnが出ていますので、これは直してください。
お気に入りタグの登録/解除が既に動作していることを考えると、propTypeをstringにして、prop名をtagにしたほうがきれいかなと思います。
また、キーワードミュートが動作していないように見えます。
image

@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 5, 2017 •
edited
Loading

idはstringでしたね失礼しました。

正規表現フィルタは tootsuite#4225で自分の投稿は必ず表示したままという仕様に変わったはずなのでこれで動いていると思います。違うアカウントに切り替えてやってみてください。

@takayamaki
Copy link
Copy Markdown
Member

takayamaki commented Dec 5, 2017

ColumnSettingsコンポーネント、SettingToggleコンポーネント、SettingTextコンポーネントにpropsのtagが流れてきていません。
/app/javascript/mastodon/features/hashtag_timeline/index.js および各コンテナの修正漏れだと思われます。

image

@fvh-P fvh-P force-pushed the TagTL-setting branch 3 times, most recently from fedd1af to a279553 Compare December 5, 2017 08:53
@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 5, 2017

これでどうでしょう。

@takayamaki
Copy link
Copy Markdown
Member

takayamaki commented Dec 5, 2017

propsのエラーは解消されました。

しかし、ミュートについては2つめのユーザを作成して試してみましたがハッシュタグTLのもののみ引き続き動作しませんでした。
image

@fvh-P
Copy link
Copy Markdown
Collaborator Author

fvh-P commented Dec 6, 2017

まだtagIdのまま残ってるところがいくつかありました。おそらくそれの影響かと思います。

takayamaki
takayamaki approved these changes Dec 6, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@takayamaki takayamaki left a comment

Choose a reason for hiding this comment

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

LGTM
image

@takayamaki takayamaki merged commit 0244a83 into imas:imastodon Dec 6, 2017
takayamaki added a commit to takayamaki/mastodon that referenced this pull request Dec 17, 2017
@takayamaki
英大文字のお気に入りタグから英小文字のstreamingを開くようにしていたcommit( mastodon#64 )の戻し
7f7bba3 
- toLowerCaseが原因で imas#96 が期待する動作をしていないため。
- toLowerCaseを付加した理由のバグが mastodon#4804 にて修正済みのため。
@takayamaki takayamaki mentioned this pull request Dec 17, 2017
Merged
takayamaki added a commit that referenced this pull request Dec 18, 2017
@takayamaki
英大文字のお気に入りタグから英小文字のstreamingを開くようにしていたcommit( #64 )の戻し
e9cf8fb 
- toLowerCaseが原因で #96 が期待する動作をしていないため。
- toLowerCaseを付加した理由のバグが mastodon#4804 にて修正済みのため。
@fvh-P fvh-P deleted the TagTL-setting branch February 6, 2018 08:11
@takayamaki takayamaki mentioned this pull request Sep 17, 2018
Merged
@lnanase lnanase mentioned this pull request Nov 28, 2018
Merged
@lnanase lnanase mentioned this pull request May 29, 2019
Merged
takayamaki pushed a commit that referenced this pull request Jun 6, 2019
@lnanase
update merge to im@stodon hashtag limeline
0803038 
c.f. #96
@lnanase lnanase mentioned this pull request Jun 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@takayamaki takayamaki takayamaki approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fvh-P @takayamaki