Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
96aba17
非Reactに書き換える
smallmonkeykey Sep 15, 2025
7afa975
不要なReactコンポーネントファイルを削除
smallmonkeykey Sep 15, 2025
2fd21e1
ブックマーク削除処理をCurrentUser::BookmarksControllerに移動
smallmonkeykey Sep 16, 2025
c3c5e8d
lint実行し忘れによる修正
smallmonkeykey Sep 16, 2025
15a753e
ページネーションで次のページにある時にも対応
smallmonkeykey Sep 16, 2025
5ede14e
ブックマーク削除処理のバグ修正
smallmonkeykey Sep 16, 2025
86a09d1
削除後に編集ボタンが効いてなかったので修正した
smallmonkeykey Sep 17, 2025
bc73f0c
余分なスペースを削除
smallmonkeykey Sep 17, 2025
f1e0b82
リンクが存在しない環境での不安定化を避けるため、存在確認を入れてからクリックする
smallmonkeykey Sep 17, 2025
c72f92b
N+1クエリ回避のためにuserを事前読込と未使用インスタンス変数の削除
smallmonkeykey Sep 17, 2025
a1087a0
ビューを整理(i18nにmodel_name.humanを使用、URLをpathに変更)
smallmonkeykey Sep 17, 2025
acaa488
request.jsを使って書き換えた
smallmonkeykey Sep 17, 2025
f1f58e2
エラーの表示を変更
smallmonkeykey Sep 17, 2025
857481b
件数を統一した
smallmonkeykey Sep 17, 2025
ca411ec
destroyのレスポンスを204no_contentに統一
smallmonkeykey Sep 18, 2025
d75047e
ブックマーク削除処理のリファクタリング
smallmonkeykey Sep 18, 2025
5ce49bb
テストが通るようにテストを変更
smallmonkeykey Sep 18, 2025
574b74e
ブックマーク削除時に部分更新ではなく全体を再描画するよう変更
smallmonkeykey Sep 18, 2025
e2e769c
ページネーションを増やしたので、上にあるのをクリックするように該当のテストを変更
smallmonkeykey Sep 18, 2025
611e8a9
不要なデバッグを削除とエラーメッセージにHTTPステータスコードを含めた
smallmonkeykey Sep 20, 2025
2be8774
link_toからbuttonに変更したのと、インデントを整えた
smallmonkeykey Sep 20, 2025
31525e0
重複している箇所をset_bookmarksでまとめた
smallmonkeykey Sep 29, 2025
84c32e9
削除後も編集ボタンを使えるように変更
smallmonkeykey Sep 30, 2025
57d8590
private書き忘れ
smallmonkeykey Sep 30, 2025
5466235
重複処理をbookmarks-utils.jsに切り出し
smallmonkeykey Sep 30, 2025
f87191a
preloadの方が効率的なため変更
smallmonkeykey Sep 30, 2025
485360d
JSらしい書き方に変更
smallmonkeykey Sep 30, 2025
8323a60
Prettierによるbookmarks関連ファイルの整形
smallmonkeykey Oct 1, 2025
82c8adb
存在チェックの不備とループ処理の一貫性を改善した
smallmonkeykey Oct 1, 2025
be84b10
不要なデバッグとouterHTMLに変更
smallmonkeykey Oct 1, 2025
e5c847a
重複していた箇所を削除
smallmonkeykey Oct 1, 2025
2dc4182
nullチェックが不足していたため修正
smallmonkeykey Oct 1, 2025
f36b854
削除後の動作を修正
smallmonkeykey Oct 14, 2025
0259b06
リファクタリングした
smallmonkeykey Oct 15, 2025
1fb26ab
JSファイルを修正
smallmonkeykey Oct 15, 2025
45ab3bf
`@rails/request.js`のgetに置き換え
smallmonkeykey Oct 15, 2025
f138a9d
セッションで値を編集モードの値を管理し次のページに言っても編集モードを維持する
smallmonkeykey Nov 11, 2025
6478092
ブックマーク編集モードの初期化処理を整理
smallmonkeykey Nov 11, 2025
3e36df6
ページに再度アクセスしたときに編集モードから始まっていたのでページから離れる時は削除
smallmonkeykey Nov 11, 2025
a9c5757
空ページのフォールバックで特殊なためコメントを追加
smallmonkeykey Nov 11, 2025
f5ecbac
条件付きを削除し、元のコードに戻した
smallmonkeykey Nov 11, 2025
2cec738
元のテストコードに戻した
smallmonkeykey Nov 11, 2025
20780f4
2ページめに相談部屋があるので2ページめに飛ぶようにした
smallmonkeykey Nov 11, 2025
be42876
1ページめ2ページめにあってもどちらでも対応できるように変更
smallmonkeykey Nov 11, 2025
25cbf94
lint修正
smallmonkeykey Nov 12, 2025
b24fb2f
置き換え後に削除ボタンが反応しなくなる問題を、親要素にイベントを渡すことで修正
smallmonkeykey Nov 19, 2025
c48ac75
ブックマーク編集のリファクタリング 命名と定数を整理
smallmonkeykey Dec 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/api/bookmarks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

class API::BookmarksController < API::BaseController
PAGER_NUMBER = 25
PAGER_NUMBER = 20

def index
per = params[:per] || PAGER_NUMBER
Expand Down
17 changes: 15 additions & 2 deletions app/controllers/current_user/bookmarks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
# frozen_string_literal: true

class CurrentUser::BookmarksController < ApplicationController
def index
@user = current_user
PAGER_NUMBER = 20

before_action :set_bookmarks, only: [:index]

def index; end

def destroy
current_user.bookmarks.find(params[:id]).destroy
head :no_content
end

private

def set_bookmarks
@bookmarks = current_user.bookmarks.preload(bookmarkable: :user).order(created_at: :desc, id: :desc).page(params[:page]).per(PAGER_NUMBER)
end
end
6 changes: 6 additions & 0 deletions app/javascript/bookmarks-delete-button-visibility.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export function toggleDeleteButtonVisibility(editToggle, deleteButtons) {
const displayStyle = editToggle.checked ? 'block' : 'none'
for (const button of deleteButtons) {
button.style.display = displayStyle
}
}
23 changes: 0 additions & 23 deletions app/javascript/bookmarks-edit-button.js

This file was deleted.

83 changes: 83 additions & 0 deletions app/javascript/bookmarks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { get, destroy } from '@rails/request.js'
import { toggleDeleteButtonVisibility } from './bookmarks-delete-button-visibility'

const EDIT_MODE_KEY = 'bookmark_edit_mode'
const DELETE_BUTTON_CLASS = 'js-bookmark-delete-button'

document.addEventListener('DOMContentLoaded', () => {
const editButton = document.getElementById('bookmark_edit')
const pageBody = document.querySelector('.page-body')
if (!pageBody) return

const savedValue = sessionStorage.getItem(EDIT_MODE_KEY)
const savedMode = savedValue === 'true'
initialize(savedMode)

if (editButton) {
editButton.addEventListener('change', () => {
const deleteButtons = document.getElementsByClassName(DELETE_BUTTON_CLASS)
toggleDeleteButtonVisibility(editButton, deleteButtons)
sessionStorage.setItem(EDIT_MODE_KEY, editButton.checked)
})
}

document.addEventListener('click', async (event) => {
const deleteButton = event.target.closest('.bookmark-delete-button')
if (!deleteButton) return

deleteButton.disabled = true

try {
const url = deleteButton.dataset.url
const response = await destroy(url)

if (!response.ok) {
throw new Error(`削除に失敗しました。(ステータス: ${response.status})`)
}

const params = new URLSearchParams(location.search)
const currentPage = parseInt(params.get('page') || '1', 10)
const newPageMain = await fetchPageMain(currentPage)

// 空ページの場合は1ページ前にフォールバック
let pageToShow = newPageMain
if (currentPage > 1 && newPageMain.querySelector('.o-empty-message')) {
pageToShow = await fetchPageMain(currentPage - 1)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

バグに気づいていただいてありがとうございます🙏

kaminariの空ページ問題とその対処について初めて知りました💡

このままでもよいと思いますが、空ページのフォールバックであることがわかるようなメソッド名をつけてロジックをそのまま切り出したり、もしくはコメントを一言添えるだけでもより読みやすくなると思いました。

const pageToShow = await メソッド()
document.querySelector('.page-body').replaceWith(pageToShow)


const fetchPageMain = async (page) => {
}

const メソッド = async () => {
  // fetchPageMainはこの中で呼び出す(デコレーターパターン)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

フォールバックという単語をはじめて知りました👀
教えてくださってありがとうございます!

こちらでコメントをつける修正をしました。
b14e410

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

私もはじめて使ったかもしれません!

コメント追加ありがとうございます🙏

document.querySelector('.page-body').replaceWith(pageToShow)

const savedModeAfterDelete =
sessionStorage.getItem(EDIT_MODE_KEY) === 'true'
initialize(savedModeAfterDelete)
} catch (error) {
console.warn(error)
deleteButton.disabled = false
}
})
})

window.addEventListener('beforeunload', () => {
const isBookmarkPage = location.pathname.includes('/current_user/bookmarks')
if (!isBookmarkPage) {
sessionStorage.removeItem(EDIT_MODE_KEY)
}
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

特定の要素とかではなくpathで判定するのは確実だしわかりやすいですね!👍

const fetchPageMain = async (page) => {
const bookmarkUrl = `/current_user/bookmarks?page=${page}`
const response = await get(bookmarkUrl, { responseKind: 'html' })
const html = await response.text
const parser = new DOMParser()
const parsedDocument = parser.parseFromString(html, 'text/html')
return parsedDocument.querySelector('.page-body')
}

const initialize = (deleteMode = false) => {
const editButton = document.getElementById('bookmark_edit')
const deleteButtons = document.getElementsByClassName(DELETE_BUTTON_CLASS)

if (editButton && deleteButtons.length > 0) {
editButton.checked = deleteMode
toggleDeleteButtonVisibility(editButton, deleteButtons)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

initializerの設置ありがとうございます🙏

次の2点が気になりました。

  1. 要素取得&イベントリスナー設定の重複
    削除ボタンの方はDOMの更新の度に再取得が必要ですが、編集ボタンの方はDOMContentLoadedの直下で取得すれば1度で済むので、DRYにできるかと思います。

  2. 責務の混在
    線引きには絶対的な正解はないと思うのですが、下図のようにUI初期化/アップデートと、編集ボタンのクリックイベントを分けると、より責務が明確になると思いました。

DOMContentLoaded () {
  // ここで編集ボタンを取得・イベントリスナー設定
  
  function initializer() {
    // クロージャで編集ボタンにアクセス
  }
  
  function handleEditToggleChange() {
    // クロージャで編集ボタンにアクセス
  }
  
  initializer() // 初回実行
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

教えてくださり、ありがとうございます!
1について重複の箇所を直しました。
2についてですが、なるべく教えていただいた通りの順番なるように、コードなどもまとめてみました。
めちゃくちゃ個人的な感覚で申し訳ないのですが、initializerが2回あるので中にfunctionがあるよりもconstにしたほうが読みやすいかなと思い、外にださせてもらっています🙇‍♂️

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

修正ありがとうございます!

めちゃくちゃ個人的な感覚で申し訳ないのですが

いいえ!💦 私も確証が持てないままベストエフォートでコメントしている感じなので、自分がこう書きたいということがあればぜひ遠慮なく言ってください👍

240 changes: 0 additions & 240 deletions app/javascript/components/Bookmarks.jsx

This file was deleted.

2 changes: 1 addition & 1 deletion app/javascript/packs/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import '../learning-completion-message.js'
import '../choices-ui.js'
import '../training-info-toggler.js'
import '../welcome_message_for_adviser.js'
import '../bookmarks-edit-button.js'
import '../bookmarks.js'
import '../hibernation_agreements.js'
import '../current-date-time-setter.js'
import '../modal-switcher.js'
Expand Down
Loading