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: improve error message when a component is moved from one UI to another. #18019

Merged
merged 5 commits into from Nov 13, 2023

Conversation

mvysny
Copy link
Member

@mvysny mvysny commented Nov 13, 2023

Description

Improve error message when a component is moved from one UI to another.

Fixes #9376

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

Sister function for Element.get(StateNode), but returns an empty Optional instead of
throwing an exception
@mcollovati mcollovati changed the title fix: 9376 improve error message when a component is moved from one UI to another. fix: improve error message when a component is moved from one UI to another. Nov 13, 2023
Copy link

github-actions bot commented Nov 13, 2023

Test Results

1 040 files  ±  0  1 040 suites  ±0   1h 8m 25s ⏱️ -23s
6 680 tests +  3  6 635 ✔️ +3  45 💤 ±0  0 ±0 
6 979 runs   - 10  6 925 ✔️  - 8  54 💤  - 2  0 ±0 

Results for commit c5757d0. ± Comparison against base commit 1c519a6.

♻️ This comment has been updated with latest results.

@mcollovati
Copy link
Collaborator

Some minor changes requested, but LGTM. Great job @mvysny

@mvysny
Copy link
Member Author

mvysny commented Nov 13, 2023

Thank you @mcollovati , all items should now be resolved.

Copy link

sonarcloud bot commented Nov 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@czp13 czp13 left a comment

Choose a reason for hiding this comment

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

Looks good to me as well, nice work @mvysny!

@mvysny mvysny merged commit 1968bed into main Nov 13, 2023
26 checks passed
@mvysny mvysny deleted the fix/9376 branch November 13, 2023 10:55
vaadin-bot pushed a commit that referenced this pull request Nov 17, 2023
…nother. (#18019)

* feat: Add ElementUtil.from(StateNode) which returns Optional<Element>

Sister function for Element.get(StateNode), but returns an empty Optional instead of
throwing an exception

* fix: improve error message when a component is moved from one UI to another.

Fixes #9376

* chore: mention both createLocation and attachLocation in the exception message

* chore: mention both createLocation and attachLocation in the exception message

* chore: minor fixes in messages
@vaadin-bot
Copy link
Collaborator

Hi @mvysny and @mvysny, when i performed cherry-pick to this commit to 24.1, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 1968bed
error: could not apply 1968bed... fix: improve error message when a component is moved from one UI to another. (#18019)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

mcollovati pushed a commit that referenced this pull request Nov 17, 2023
…nother. (#18019)

* feat: Add ElementUtil.from(StateNode) which returns Optional<Element>

Sister function for Element.get(StateNode), but returns an empty Optional instead of
throwing an exception

* fix: improve error message when a component is moved from one UI to another.

Fixes #9376

* chore: mention both createLocation and attachLocation in the exception message

* chore: mention both createLocation and attachLocation in the exception message

* chore: minor fixes in messages
vaadin-bot added a commit that referenced this pull request Nov 17, 2023
…nother. (#18019) (CP: 24.2) (#18060)

* fix: improve error message when a component is moved from one UI to another. (#18019)

* feat: Add ElementUtil.from(StateNode) which returns Optional<Element>

Sister function for Element.get(StateNode), but returns an empty Optional instead of
throwing an exception

* fix: improve error message when a component is moved from one UI to another.

Fixes #9376

* chore: mention both createLocation and attachLocation in the exception message

* chore: mention both createLocation and attachLocation in the exception message

* chore: minor fixes in messages

* test: reset ComponentTracker.productionMode static field (#18028)

Prevents unexpected situation in ComponentTest caused by the
ComponenTracker.productionMode flag being initialized to true
by some other tests.
In particular, cannotMoveComponentsToOtherUI test may fail because
the exception message is different in development and production
modes.

---------

Co-authored-by: Martin Vysny <mavi@vaadin.com>
Co-authored-by: Marco Collovati <marco@vaadin.com>
mcollovati added a commit that referenced this pull request Nov 17, 2023
…nother. (#18019) (CP: 24.1) (#18061)

* fix: improve error message when a component is moved from one UI to another. (#18019)

* feat: Add ElementUtil.from(StateNode) which returns Optional<Element>

Sister function for Element.get(StateNode), but returns an empty Optional instead of
throwing an exception

* fix: improve error message when a component is moved from one UI to another.

Fixes #9376

* chore: mention both createLocation and attachLocation in the exception message

* chore: mention both createLocation and attachLocation in the exception message

* chore: minor fixes in messages

* test: reset ComponentTracker.productionMode static field (#18028)

Prevents unexpected situation in ComponentTest caused by the
ComponenTracker.productionMode flag being initialized to true
by some other tests.
In particular, cannotMoveComponentsToOtherUI test may fail because
the exception message is different in development and production
modes.

---------

Co-authored-by: Martin Vysny <mavi@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add clarifying information to the error message about moving nodes to another state tree
4 participants