Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: recreate dropdown div #7572

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Conversation

potaracom
Copy link
Contributor

The basics

The details

Resolves

Fixes #7571

Proposed Changes

Check if the element(.blocklyDropDownDiv) exists.

Reason for Changes

Closing blockly does not reset the 'div' variable.
Therefore, the createDom method of dropDownDiv is not re-executed.

Test Coverage

I have verified that the drop-down elements are displayed on the second view.

Documentation

Additional Information

@potaracom potaracom requested a review from a team as a code owner October 6, 2023 07:14
@github-actions github-actions bot added the PR: fix Fixes a bug label Oct 6, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome! It looks like this is your first pull request in Blockly, so here are a couple of tips:

  • You can find tips about contributing to Blockly and how to validate your changes on our developer site.
  • All contributors must sign the Google Contributor License Agreement (CLA). If the google-cla bot leaves a comment on this PR, make sure you follow the instructions.
  • We use conventional commits to make versioning the package easier. Make sure your commit message is in the proper format or learn how to fix it.
  • If any of the other checks on this PR fail, you can click on them to learn why. It might be that your change caused a test failure, or that you need to double-check the style guide.
    Thank you for opening this PR! A member of the Blockly team will review it soon.

@BeksOmega BeksOmega requested review from BeksOmega and removed request for cpcallen October 6, 2023 18:56
@BeksOmega BeksOmega requested review from maribethb and removed request for BeksOmega October 6, 2023 19:28
@BeksOmega BeksOmega requested review from BeksOmega and removed request for maribethb October 6, 2023 19:29
@BeksOmega
Copy link
Collaborator

Hiya @potaracom Thank you so much for putting up this PR! I looks great =)

I think this also fails for the tooltip and widget div, could you test for those as well, and fix them if that's the case?

@potaracom
Copy link
Contributor Author

@BeksOmega
I fixed it.

before

before

after

after

@potaracom
Copy link
Contributor Author

I'm sorry. I will fix test.

@BeksOmega
Copy link
Collaborator

Hiya @potaracom ! Thank you for fixing the tests :D I think there might be a simpler solution though. If you remove the call to testOnly_setDiv and its definition does that fix the problem?

If there was another reason (besides the tests) that made you want to remove the global variables, just let me know!

Thank you for your hard work on this :D

@potaracom
Copy link
Contributor Author

@BeksOmega
There is no reason to remove global variables.
Changed testOnly_setDiv.

Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

This LGTM! Just going to wait for CI to pass and then I'll get this merged!

Let me know if you'd like help finding something else to work on (if you're interested!).

Thank you again for putting up a fix for this =)

@BeksOmega BeksOmega merged commit 1e3b5b4 into google:develop Oct 10, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

blockly redisplay does not show dropdown elements
4 participants