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

[api-samples] Use dynamic label api-sample only when activated #9517

Merged

Conversation

gbodeen
Copy link
Contributor

@gbodeen gbodeen commented May 25, 2021

Signed-off-by: Gabriel Bodeen [email protected]

What it does

Fixes #6796. As the comment on the issue says, the problem is caused by the dynamic label example in the api-samples package, so it should only happen in the Theia repo and not products built using Theia. Nevertheless it's undesirable behavior.

How to test

  1. Open the search-in-workspace widget & enter search text as well as replacement text. Click one of the elements in the tree to open the diff editor. The editor tab title should be visible.
    • The api sample checks for the presence of the word "test" in the URI, which for a diff editor includes the whole contents of the file after replacement. So for testing this way to work, you must open a diff editor to a file that has the word "test" (case sensitive) in it somewhere, but be sure you don't strip out that word from the diff using the search-and-replace.
  2. Use the command palette to activate Examples: Toggle Dynamically-Changing Labels. The editor tab title should be replaced with periodically updating numbers. Run the command again, and the original tab title will be restored.

dynamic-label

Review checklist

Reminder for reviewers

@gbodeen
Copy link
Contributor Author

gbodeen commented May 25, 2021

BTW, let me know if you prefer larger PRs that address multiple related issues. I usually keep it to a single issue so that the change is super obvious, but I understand that can also irk some folk when a bunch of tiny changes pile up. 😁

@vince-fugnitto
Copy link
Member

BTW, let me know if you prefer larger PRs that address multiple related issues. I usually keep it to a single issue so that the change is super obvious, but I understand that can also irk some folk when a bunch of tiny changes pile up.

@gbodeen better a pull-request fixes a single issue unless issues can be easily grouped together (and even then we may want to still have multiple commits) 😃

@colin-grant-work colin-grant-work merged commit 151cf8c into eclipse-theia:master May 26, 2021
@vince-fugnitto vince-fugnitto added this to the 1.14.0 milestone May 27, 2021
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.

[search] opened diff editors don't have a name
3 participants