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

Layout fixes #6066

Merged
merged 7 commits into from
Mar 25, 2023
Merged

Layout fixes #6066

merged 7 commits into from
Mar 25, 2023

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented Mar 23, 2023

Pull Request Description

This PR contains fixes for several bugs found by me and @Frizi during development of our current tasks. Created unit tests for each case (and many more).

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@farmaazon farmaazon self-assigned this Mar 23, 2023
@farmaazon farmaazon requested a review from wdanilo March 23, 2023 16:09
@farmaazon
Copy link
Contributor Author

farmaazon commented Mar 23, 2023

Created as draft, as I still need to test if changes does not break something in IDE. Besides, it's ready for review.

@farmaazon farmaazon added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 23, 2023
@farmaazon farmaazon force-pushed the wip/farmaazon/layout-fixes branch from 5a314d6 to 8dc91f0 Compare March 23, 2023 16:49
@farmaazon farmaazon force-pushed the wip/farmaazon/layout-fixes branch from 8dc91f0 to 289714d Compare March 23, 2023 16:54
@farmaazon farmaazon marked this pull request as ready for review March 23, 2023 17:04
Comment on lines 3070 to 3073
if self.dirty.transformation.check()
|| self.dirty.modified_children.check_all()
|| self.dirty.removed_children.check_all() =>
0.0,
Copy link
Member

Choose a reason for hiding this comment

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

Please check if it is possible to minimize the amount of checks, as we agreed via phone.

Comment on lines 3081 to 3084
if self.dirty.transformation.check()
|| self.dirty.modified_children.check_all()
|| self.dirty.removed_children.check_all()
{
Copy link
Member

Choose a reason for hiding this comment

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

could you please refactor that to one helper fn?

@farmaazon farmaazon requested a review from wdanilo March 24, 2023 15:15
@Frizi Frizi requested a review from mwu-tow as a code owner March 24, 2023 20:12
Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

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

@farmaazon and @Frizi amazing job with debugging and fixing it! I'm sorry I did not spot these issues when I was developing the code.

@wdanilo wdanilo merged commit 866a58a into develop Mar 25, 2023
@wdanilo wdanilo deleted the wip/farmaazon/layout-fixes branch March 25, 2023 01:28
@wdanilo
Copy link
Member

wdanilo commented Mar 25, 2023

Merged, as we need it to develop the codebase, and the CI problems seem unrelated.

Procrat added a commit that referenced this pull request Mar 27, 2023
* develop:
  Layout fixes (#6066)
  Use new Enso Hash Codes and Comparable (#6060)
  Search suggestions by static attribute (#6036)
  Use .node-version for pinning Node.js version (#6057)
  Fix code generation for suggestion preview (#6054)
  Implementation of EnsoGL predefined Rectangle shape. (#6033)
  Tidy up the public module level statics (#6032)
  Cursor aware Component Browser (#5770)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants