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

Modernized help UI prototype: Fix set scope still set after deletion #735

Merged
merged 1 commit into from
May 8, 2024

Conversation

howlger
Copy link
Contributor

@howlger howlger commented Oct 9, 2023

Fix the issue that a set scope will still be shown as active after it has been deleted.

@akurtakov
Copy link
Member

@howlger Do you intent to make the new one default or at least have a UI preference to be able to switch to it ?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

Test Results

 1 731 files  ±0   1 731 suites  ±0   1h 29m 4s ⏱️ + 3m 45s
 4 005 tests ±0   3 983 ✅ ±0   22 💤 ±0  0 ❌ ±0 
12 612 runs  ±0  12 451 ✅ ±0  161 💤 ±0  0 ❌ ±0 

Results for commit cb42a00. ± Comparison against base commit 6a0853d.

♻️ This comment has been updated with latest results.

@howlger
Copy link
Contributor Author

howlger commented Oct 9, 2023

@howlger Do you intent to make the new one default or at least have a UI preference to be able to switch to it ?

Active help is not yet supported. Should that be done first or should we already add a checkbox to enable the modernized help UI first?

The test failure on Windows does not seem to me to be related to this change.

@akurtakov
Copy link
Member

@howlger Do you intent to make the new one default or at least have a UI preference to be able to switch to it ?

Active help is not yet supported. Should that be done first or should we already add a checkbox to enable the modernized help UI first?

Being able to switch to it even without active help would be beneficial.

The test failure on Windows does not seem to me to be related to this change.

@laeubi
Copy link
Contributor

laeubi commented Oct 9, 2023

By the way I don't know how active help is implemented but I have created a while back

This would allow some more active features (not only in help) to be performed when interacting with the "host code" as then one can implement some rest-like api to interact with the embedding code using JS in the browser.

@laeubi
Copy link
Contributor

laeubi commented Oct 9, 2023

So in the given "Active Help Example" there is currently a link "Open a cheatsheet" if I click on this I get a feedback that I need to run this "local" (whatever this should mean).

With AJAX intercept, the code could b rewritten:

  1. The link (or even some textblocks or buttons or whatever Ui elemnt) will be hidden
  2. The the JS code will make an AJAX request to for example activeAction.json?id =org.eclipse.ui.cheatsheets.OpenCheatSheetFromHelpAction what will be answered by a static json like {activeActions:false} what would result in keep the non-functional UI hidden
  3. in "local" mode the call will be intercepted and instead one will get a json like this {activeActions:true, payload={ ... }}
  4. This will then show the UI and on click perform a POST activeAction.json with given payload to the url
  5. This can then again be intercepted and perform any action in Java code one desires.

@howlger
Copy link
Contributor Author

howlger commented Oct 10, 2023

@laeubi Thanks for the link. Currently, the modernized help UI sits on top of the legacy help. Things like displaying search results, which should be better done on server side, are currently done via JavaScript in the web browser. The active help does not yet work because it is bound to the HTML frameset structure of the legacy help. It's probably easy to fix, but it's not done yet.

See the modernized help UI e.g. here vs. deactivated with the parameter legacy.

So in the given "Active Help Example" there is currently a link "Open a cheatsheet" if I click on this I get a feedback that I need to run this "local" (whatever this should mean).

The active help works only in Eclipse or in an Eclipse-based application, accessible via Help > Help Contents, not in the stand-alone Information Center (e.g. help.eclipse.org). The idea is to provide links to open a specific dialog or to execute other commands.

Copy link
Member

@iloveeclipse iloveeclipse left a comment

Choose a reason for hiding this comment

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

In order to understand & review the change, another change is needed first, see my comment in code.

ua/org.eclipse.help.webapp/m/index.js Outdated Show resolved Hide resolved
howlger added a commit to howlger/eclipse.platform that referenced this pull request Oct 10, 2023
Refactor JavaScript code for better readability. No functional changes.

See also eclipse-platform#735 (review)
howlger added a commit to howlger/eclipse.platform that referenced this pull request Oct 13, 2023
Refactor JavaScript code for better readability. No functional changes.

See also eclipse-platform#735 (review)
howlger added a commit to howlger/eclipse.platform that referenced this pull request Oct 13, 2023
Refactor JavaScript code for better readability. No functional changes.

See also eclipse-platform#735 (review)
iloveeclipse pushed a commit to howlger/eclipse.platform that referenced this pull request May 8, 2024
Refactor JavaScript code for better readability. No functional changes.

See also eclipse-platform#735 (review)
iloveeclipse pushed a commit that referenced this pull request May 8, 2024
Refactor JavaScript code for better readability. No functional changes.

See also #735 (review)
Fix the issue that a set scope will still be shown as active after it
has been deleted.
@iloveeclipse iloveeclipse merged commit 50a95d9 into eclipse-platform:master May 8, 2024
16 checks passed
@iloveeclipse
Copy link
Member

Danke Holger, sorry ist bei mir leider untergegangen.

@howlger
Copy link
Contributor Author

howlger commented May 10, 2024

Danke Holger, sorry ist bei mir leider untergegangen.

Thanks for reviewing and merging.

It would also have been up to me to ask. Hopefully I will soon find some time to work on what @akurtakov says.

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.

4 participants