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.

Sign up for GitHub

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

Jump to bottom

ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す #142

Merged
merged 3 commits into from
Dec 22, 2016
Merged

ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す #142

merged 3 commits into from
Dec 22, 2016

Conversation

hattori6789
Copy link
Contributor

@hattori6789 hattori6789 commented Dec 7, 2016 •
edited
Loading

connected to #141

@komagata komagata temporarily deployed to interns-com-pr-142 December 7, 2016 01:52 Inactive
@komagata komagata temporarily deployed to interns-com-pr-142 December 7, 2016 03:00 Inactive
@hattori6789 hattori6789 changed the title [WIP]ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す Dec 7, 2016
@hattori6789
Copy link
Contributor Author

hattori6789 commented Dec 7, 2016 •
edited
Loading

参考画像

修正前
default

@komagata komagata temporarily deployed to interns-com-pr-142 December 7, 2016 03:19 Inactive
@hattori6789
Copy link
Contributor Author

@komagata @machida レビューお願いします。
ログインしていないユーザーの場合、検索フォームを隠しました。

参考画像

修正後
2016-12-07 12 09 17

@komagata
Copy link
Member

komagata commented Dec 8, 2016

ログインしてるかどうかというのはヘルパーがあったはず。

hattori6789
hattori6789 commented Dec 8, 2016
View reviewed changes
app/views/application/_header.html.slim
.global-nav-links.is-contents-links
ul.global-nav-links__items
- if current_user
- if current_user
Copy link
Contributor Author

@hattori6789 hattori6789 Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@komagata こちらの行でユーザーのログイン状態を判定出来ているつもりでいました。current_userではない処理を何か記述した方が良い、ということであっていますか？

ログインしてるかどうかというのはヘルパーがあったはず。

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
Contributor Author

@hattori6789 hattori6789 Dec 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@komagata app/helpers/以下のファイルや、ビューヘルパーというものに関することかと思い調べてみたのですが分かりませんでした。もし宜しければ、どこで定義されているのか、どこを見れば良いのかなどアドバイス頂きたいです。

@hattori6789
Copy link
Contributor Author

hattori6789 commented Dec 8, 2016 •
edited
Loading

@komagata 検索フォームに、検索したワードを表示するようにしました。（修正箇所）
PRタイトルと矛盾する作業なので迷ったのですが、修正箇所が同じ場所であったこと、小さな変更であったため、コミットに含めましたが分けた方が良かったでしょうか。

参考画像

修正前

2016-12-08 17 53 35

修正後

2016-12-08 18 07 18

@komagata
Copy link
Member

@hattori6789 sorceryのREADMEをみてみてください。

@hattori6789
Copy link
Contributor Author

@komagata 確認します。ありがとうございます！

@komagata komagata force-pushed the master branch 3 times, most recently from 20f1999 to 6939136 Compare December 16, 2016 10:44
@hattori6789 hattori6789 changed the title ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す [WIP]ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す Dec 19, 2016
@hattori6789 hattori6789 changed the title [WIP]ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す ログインユーザーでない場合、ホーム画面のヘッダーから検索フォームを隠す Dec 19, 2016
@hattori6789
Copy link
Contributor Author

@komagataレビューお願いします！
current_userをlogged_in?へ変更しました。

@hattori6789 hattori6789 self-assigned this Dec 20, 2016
@komagata
Copy link
Member

@hattori6789 LGTM

@komagata komagata merged commit eb8baa4 into master Dec 22, 2016
@komagata komagata deleted the hide-search-form branch December 22, 2016 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@komagata komagata komagata left review comments

Assignees

@hattori6789 hattori6789

Labels
新機能
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hattori6789 @komagata