Skip to content

Make it easier to reorder AssetGroup and AssetFilter.#67

Merged
hkmt-mmy merged 1 commit intomainfrom
feature/reorder
Mar 17, 2025
Merged

Make it easier to reorder AssetGroup and AssetFilter.#67
hkmt-mmy merged 1 commit intomainfrom
feature/reorder

Conversation

@hkmt-mmy
Copy link
Copy Markdown
Contributor

@hkmt-mmy hkmt-mmy commented Mar 13, 2025

#64

  • AssetGroupとAssetFilterを対象に改修
  • 右クリック時のメニューに「Move Up By」「Move Down By」を追加
    • 既存の「Move Up」「Move Down」とは別にする
    • 子要素として移動可能な量を表示する。index = 3の要素だとすると 1, 2, 3 の3要素
    • 移動不可能な場合は親要素である「Move Up By」または「Move Down By」自体を無効化する
  • ロジックとしては Move Up は Move Up By 1、Move Down は Move Down By 1 として共通化する

確認事項

  • リストの先頭にある場合、Move Up By の項目が無効化される
  • リストの末尾にある場合、Move Down By の項目が無効化される
  • 要素3つの真ん中にある場合、Move Up ByおよびMove Down Byそれぞれに「1」の子要素がある
  • 要素4つの2つめにある場合、Move Up Byには「1」、Move Down Byには「1」「2」の子要素がある
  • Move Up By の移動量が指定した数値通りになる (例えば2なのに1だけ移動になったりしない)
  • Move Down By の移動量が指定した数値通りになる
  • 移動を複数回繰り返しても、移動可能量が正常に計算される

@hkmt-mmy hkmt-mmy requested review from Haruma-K and Copilot March 13, 2025 09:24
@hkmt-mmy hkmt-mmy linked an issue Mar 13, 2025 that may be closed by this pull request
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces functionality to allow reordering of AssetGroup and AssetFilter items by specific offsets via new "Move Up By" and "Move Down By" menu options.

  • Added new events and methods to compute movable options based on the current index.
  • Updated UI elements (views) and undo history registration messages to support offset-based movement.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetFilterViewPresenter.cs Introduced MoveUpBy and MoveDownBy methods and hooked up new view events.
Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetGroupPanelPresenter.cs Added MoveUpByGroup and MoveDownByGroup methods along with corresponding event handlers.
Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetFilterView.cs Added new menu items and events for offset-based filter movement.
Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetGroupPanelView.cs Added new menu items, event properties, and disposable management for offset-based group movement.
Comments suppressed due to low confidence (4)

Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetFilterViewPresenter.cs:106

  • Consider adding a validation to ensure that the offset 'd' is greater than zero to guard against accidental negative moves.
private void MoveUpBy(int d)

Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetFilterViewPresenter.cs:130

  • Consider verifying that 'd' is a positive number before proceeding, similar to the check in MoveUpBy, to ensure robustness.
private void MoveDownBy(int d)

Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetGroupPanelPresenter.cs:189

  • It may be beneficial to add a check confirming that 'd' is greater than zero in MoveUpByGroup to avoid potential misuse.
void MoveUpByGroup(int d)

Assets/SmartAddresser/Editor/Core/Tools/Addresser/Shared/AssetGroups/AssetFilterView.cs:129

  • Consider adding unit tests to verify that GetMoveUpByOptions returns the correct range of options based on the current filter index.
var moveUpByList = GetMoveUpByOptions?.Invoke();

Copy link
Copy Markdown
Contributor

@Haruma-K Haruma-K left a comment

Choose a reason for hiding this comment

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

問題なさそうです、ありがとうございます。

@hkmt-mmy hkmt-mmy merged commit bab7db2 into main Mar 17, 2025
1 check passed
@hkmt-mmy hkmt-mmy deleted the feature/reorder branch March 17, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AssetGroupやAssetFilterを並び替えやすくする

3 participants