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

reactivated help button and fixed xquery parameters problems #470

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

daniel-jettka
Copy link
Contributor

Description, Context and related Issue

The help button and page was commented out, and needed some change to be displayed and trigger help window opening.

Refs #209

How Has This Been Tested?

checked manually in Klarinettenquintett and EditionExample

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement
  • Refactoring

Overview

  • I have updated the inline documentation accordingly.
  • I have performed a self-review of my code, according to the style guide
  • I have read the CONTRIBUTING document.

@daniel-jettka daniel-jettka linked an issue Nov 15, 2024 that may be closed by this pull request
Copy link
Member

@peterstadler peterstadler left a comment

Choose a reason for hiding this comment

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

While the help button works in my test with the clarinet quintet, the internal links within the help page don't work. Clicking on e.g. the link "Vorbereitung" from the help TOC opens a new browser tab with the Edirom Online index page.

add/data/xql/getHelp.xql Outdated Show resolved Hide resolved
add/data/xql/getHelp.xql Outdated Show resolved Hide resolved
@peterstadler peterstadler added this to the 1.0.0 milestone Nov 20, 2024
@daniel-jettka
Copy link
Contributor Author

While the help button works in my test with the clarinet quintet, the internal links within the help page don't work. Clicking on e.g. the link "Vorbereitung" from the help TOC opens a new browser tab with the Edirom Online index page.

This is the problem in #24 right?

Can we move the conversation there and merge this one anyway because #209 was only about reactiviating, not the content and we have the #24 ?

@peterstadler
Copy link
Member

Ups, I didn't notice #24.

But the other code comments are still valid for this PR, for those changes should be motivated.

Copy link
Member

@peterstadler peterstadler left a comment

Choose a reason for hiding this comment

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

I personally dislike all these merge-dev-into-feature-branch commits and prefer rebasing for a clean history. That said, the code changes look ok, many thanks!

@daniel-jettka daniel-jettka merged commit 2bbfb18 into develop Dec 2, 2024
4 checks passed
@daniel-jettka daniel-jettka deleted the ftr/209-reactivate-help branch December 2, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

reactivate help
4 participants