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

helpful Search only visible in advance mode #4203

Merged
merged 3 commits into from
Dec 29, 2024

Conversation

subhas-pramanik-09
Copy link
Contributor

@subhas-pramanik-09 subhas-pramanik-09 commented Dec 28, 2024

resolve #4202

The helpful search option is now only visible in advance mode

Screen.Recording.2024-12-29.014923.mp4

Copy link
Contributor

@MostlyKIGuess MostlyKIGuess left a comment

Choose a reason for hiding this comment

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

As I said, I am still able to re-create the issue. I have used docker for testing. Also what you have done in Line 503 is the same condition from line 460 and essentially adding line inside 455-457 seems pointless because we have essentially removed a condition and added it again somewhere else .

// Add event listener to remove the search div from the DOM
const modeButton = docById("begIconText");
closeButton.addEventListener("click", this._hideHelpfulSearchWidget);
modeButton.addEventListener("click", this._hideHelpfulSearchWidget);
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this even accomplish? I assume the solution is due to the missing condition from L503

@@ -498,6 +498,9 @@ class Activity {
if (docById("helpfulWheelDiv").style.display !== "none") {
docById("helpfulWheelDiv").style.display = "none";
}
if (this.helpfulSearchDiv && this.helpfulSearchDiv.parentNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this makes sense but I am still able to recreate the issue in my local system.

@subhas-pramanik-09
Copy link
Contributor Author

Yes, condition of Line 460 to 464 and Line 496 this two are same. Updated in this commit

@subhas-pramanik-09
Copy link
Contributor Author

@walterbender Sir, is this okay ?

@walterbender
Copy link
Member

Screenshot From 2024-12-29 10-33-37

Prob. we should only have one search open at a time.

@subhas-pramanik-09
Copy link
Contributor Author

Screenshot From 2024-12-29 10-33-37

Prob. we should only have one search open at a time.

That's mean if we open the master branch searchbar the helpful search should be hide ?

@walterbender
Copy link
Member

and vice versa

@subhas-pramanik-09
Copy link
Contributor Author

and vice versa

Yes Sir done. Please check

@walterbender walterbender merged commit 4e2bb77 into sugarlabs:master Dec 29, 2024
4 checks passed
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.

Helpful Search visible on canvas after switching beginner mode
3 participants