Skip to content

Do not automatically set attributes.windowWidth and attributes.windowHeight.#54

Open
EricPei20 wants to merge 1 commit into
mainfrom
fix_get_result
Open

Do not automatically set attributes.windowWidth and attributes.windowHeight.#54
EricPei20 wants to merge 1 commit into
mainfrom
fix_get_result

Conversation

@EricPei20

Copy link
Copy Markdown
Contributor

No description provided.

@CoolFanyu CoolFanyu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with Eric's change since the GetResult function will get different types of images, set the windows size to a fixed value could cause problems. DE-Server will set the window size automatically if user don't set it.

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 removes implicit client-side behavior that automatically set Attributes.windowWidth/windowHeight (and corresponding Client.width/height) when attributes is None or "auto", leaving window sizing fully controlled by the caller or the server response.

Changes:

  • Removed auto-population of attributes.windowWidth/windowHeight in get_result() for VIRTUAL_IMAGE* / EXTERNAL_IMAGE* frame types.
  • Removed pre-response overrides of self.width/self.height based on requested window sizes.
  • Deleted _get_auto_attributes() and updated get_image() to use a plain Attributes() instance when attributes == "auto".

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread deapi/client.py
Comment on lines 1595 to 1596
if attributes is None or attributes == "auto":
attributes = Attributes(**kwargs)
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.

3 participants