Skip to content

Show WButton group conflict warnings when foldouts are collapsed - #251

Merged
Eli Pinkerton (wallstop) merged 12 commits into
mainfrom
copilot/fix-wbutton-group-placement
May 8, 2026
Merged

Show WButton group conflict warnings when foldouts are collapsed#251
Eli Pinkerton (wallstop) merged 12 commits into
mainfrom
copilot/fix-wbutton-group-placement

Conversation

Copilot AI commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

Conflicting groupPlacement values were detected internally but could be effectively invisible when WButton groups started collapsed, which made misconfiguration easy to miss.
This updates warning rendering so conflicts remain visible regardless of foldout state and adds regression coverage for collapsed groups.

  • Warning visibility fix

    • Moved conflict warning rendering outside foldout content so warnings are shown for collapsed groups.
    • Keeps existing conflict resolution behavior unchanged (first declared value remains canonical).
  • Scope of conflicts covered

    • Applies consistently to all existing WButton conflict warnings:
      • draw order
      • group priority
      • group placement
  • Regression test coverage

    • Added collapsed-foldout tests in WButtonConflictWarningDisplayTests for all three warning types.
    • Added internal test accessors for warning text caches to assert warning generation in StartCollapsed mode.
// WButtonGUI.cs (conceptual flow)
DrawConflictWarnings(groupKey); // now rendered before foldout body visibility checks
if (effectiveExpanded)
{
    DrawGroupContent(groupKey, contexts, paginationStates, triggeredContexts);
}

Related Issue

Addresses the linked bug report for missing user-visible feedback on conflicting WButton group placements.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactor (code change that neither fixes a bug nor adds a feature)

Checklist

  • I have added tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have updated the CHANGELOG
  • My changes do not introduce breaking changes, or breaking changes are documented

Copilot AI changed the title [WIP] Fix conflicting placements in WButton Groups to display user warnings Show WButton group conflict warnings when foldouts are collapsed Apr 29, 2026
@wallstop
Eli Pinkerton (wallstop) marked this pull request as ready for review April 29, 2026 22:28
Copilot AI review requested due to automatic review settings April 29, 2026 22:28

Copilot AI left a comment

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.

Pull request overview

This PR fixes an Editor UX gap in the WButton UI by ensuring conflict warnings (draw order / group priority / group placement) remain visible even when WButton groups start collapsed, and adds regression tests to prevent the issue from reappearing.

Changes:

  • Render WButton conflict warnings outside the foldout body so they show even when a group is collapsed.
  • Add internal test accessors for warning text caches to enable assertions in collapsed-foldout scenarios.
  • Add regression tests covering collapsed-foldout warning visibility for placement, priority, and draw order conflicts.

Reviewed changes

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

File Description
Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs Adds regression tests asserting conflict warnings are generated/rendered when foldouts start collapsed.
Editor/Utils/WButton/WButtonGUI.cs Moves conflict warning rendering out of foldout content gating and adds internal cache accessors for tests.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs Outdated
Comment thread Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs Outdated
Comment thread CHANGELOG.md

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs
Comment thread CHANGELOG.md
Comment thread Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Comment thread package.json

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.

Comment thread Tests/Editor/WButton/WButtonConflictWarningDisplayTests.cs

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 85 out of 86 changed files in this pull request and generated 1 comment.

Comment on lines 63 to 84
check_npm_tool() {
local name="$1"
local cmd="$2"
local version_flag="${3:---version}"
local output=""
local status=0

printf "%-20s" "$name:"
if npx --no-install "$cmd" $version_flag >/dev/null 2>&1; then
version=$(npx --no-install "$cmd" $version_flag 2>&1 | head -1)
if output="$(timeout 30s npx --no-install "$cmd" $version_flag 2>&1)"; then
version=$(printf '%s\n' "$output" | head -1)
printf "${GREEN}✓${NC} %s (via npx)\n" "$version"
return 0
else
printf "${RED}✗ NOT FOUND - run 'npm install'${NC}\n"
status=$?
if [ "$status" -eq 124 ]; then
printf "${RED}✗ TIMED OUT - npx %s %s${NC}\n" "$cmd" "$version_flag"
else
printf "${RED}✗ NOT FOUND - run 'npm install'${NC}\n"
fi
FAILED=$((FAILED + 1))
return 1
fi

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 89 out of 103 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

scripts/verify-devcontainer-tools.sh:85

  • This script still validates repo-pinned npm tools via npx --no-install, but the rest of the repo has switched to invoking pinned Node CLIs through the repo-local launchers (scripts/run-prettier.js and scripts/run-node-bin.js). Using npx here can give inconsistent results (e.g., PATH/npx quirks or .bin shim behavior) and makes the verification less representative of how hooks/CI actually run these tools. Consider switching these checks to the same repo-local launchers used elsewhere.

Comment on lines 70 to 79
[SetUp]
public virtual void BaseSetUp()
{
#if REFLEX_PRESENT
EnsureReflexSettings();
#endif
#if UNITY_EDITOR
CleanupPackageRootGeneratedArtifacts();
_previousEditorUiSuppress = EditorUi.Suppress;
EditorUi.Suppress = true;
Comment on lines +84 to +96
# Validate the results path before creating the test project. Writing a Unity
# project or test output under WORKSPACE_DIR makes those generated files part of
# the imported package and can trigger Unity import-loop failures.
RESULTS_DIR="${UNITY_TEST_PROJECT_DIR}/test-results"
workspace_realpath="$(cd "${WORKSPACE_DIR}" && pwd -P)"
results_realpath="$(realpath -m "${RESULTS_DIR}")"
case "${results_realpath}" in
"${workspace_realpath}"|"${workspace_realpath}"/*)
echo "ERROR: Refusing to write Unity test results inside the package root: ${results_realpath}"
echo "ERROR: Set UNITY_TEST_PROJECT_DIR outside ${workspace_realpath} to avoid Unity package import loops."
exit 1
;;
esac
@wallstop
Eli Pinkerton (wallstop) merged commit 3616f0f into main May 8, 2026
43 checks passed
@wallstop
Eli Pinkerton (wallstop) deleted the copilot/fix-wbutton-group-placement branch May 8, 2026 19:52
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.

[Bug]: No User-Visible Error When Conflicting Placements for a WButton Groups

3 participants