Skip to content

Conversation

@plun1331
Copy link
Member

@plun1331 plun1331 commented Oct 23, 2025

Summary

This changes the way that ViewItem.view is determined.

Before:

  • View was passed between items, every item held a reference to it
    • (this also broke in the cv2 rewrite)

Now:

  • Each item only keeps track of its own parent
  • When getting the view, it recursively goes through each parent until it reaches a view type.

I have deprecated manually setting .view, though I am considering removing it outright, though that would be breaking. Open for discussion.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

@plun1331 plun1331 requested a review from a team as a code owner October 23, 2025 17:30
@plun1331 plun1331 added the bug Something isn't working label Oct 23, 2025
@pycord-app
Copy link

pycord-app bot commented Oct 23, 2025

Thanks for opening this pull request!
Please make sure you have read the Contributing Guidelines and Code of Conduct.

This pull request can be checked-out with:

git fetch origin pull/2981/head:pr-2981
git checkout pr-2981

This pull request can be installed with:

pip install git+https://github.com/Pycord-Development/pycord@refs/pull/2981/head

Copy link
Contributor

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 PR refactors how ViewItem.view is determined by removing direct view reference propagation in favor of a parent-based traversal approach. Previously, every item held a direct reference to the view that was passed and updated across items, which broke in the cv2 rewrite. Now, items only track their immediate parent and recursively traverse the parent chain to find the view when needed.

Key changes:

  • Replaced direct item._view assignments with parent-only tracking across all container types
  • Implemented recursive view resolution through the parent chain in ViewItem.view property
  • Deprecated manual .view setter with warning for removal in v2.8

Reviewed Changes

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

Show a summary per file
File Description
discord/ui/view.py Removed direct _view assignment in add_item, updated remove_item to use identity check and clear parent, fixed clear_items to properly clear parent references
discord/ui/section.py Removed _view assignment in set_accessory and deleted the custom view setter that propagated view references to child items
discord/ui/item.py Implemented recursive parent traversal in view property getter and added deprecation warning to the setter
discord/ui/container.py Removed _view assignment logic in add_item, updated remove_item to use identity check and clear parent
discord/ui/action_row.py Removed _view assignment in add_item, added parent clearing in remove_item

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@plun1331 plun1331 requested review from a team as code owners October 23, 2025 17:31
NeloBlivion
NeloBlivion previously approved these changes Oct 23, 2025
Copy link
Member

@NeloBlivion NeloBlivion left a comment

Choose a reason for hiding this comment

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

This addresses everything I was gonna pr later and then some so yeah lgtm 🙏

@plun1331 plun1331 marked this pull request as draft October 23, 2025 17:46
Copy link
Member Author

@plun1331 plun1331 left a comment

Choose a reason for hiding this comment

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

Thoughts on removing i.view = ... entirely?

@plun1331 plun1331 marked this pull request as ready for review October 23, 2025 18:13
@NeloBlivion
Copy link
Member

Thoughts on removing i.view = ... entirely?

I'm inclined to say no because then it'd actually be breaking from 2.6.1, but the deprecation itself is fine by me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants