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: selecting all pages with advanced/detail search corrected #2215

Open
wants to merge 3 commits into
base: v1.10
Choose a base branch
from

Conversation

headri
Copy link
Contributor

@headri headri commented Oct 15, 2024

this.state.currentSearchSelection is not set in detail / advanced search
therefore condition removed; the other condition is sufficient
closes https://github.com/ComPlat/chemotion/issues/217

PiTrem and others added 3 commits October 8, 2024 07:58
for example with the engine set to ^18.19.1 in package.json:  avoid selecting 20.18. instead of latest 18.xx

* also relax asdf manage nodejs version

if (range == 'all') {
if (this.state.currentSearchSelection && elements[type + "s"].ids) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

relevant change

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to be sure, the only behavioral change in this file's diff is the removal of this.state.currentSearchSelection from the guard clause? For the future please change behavior and formatting in separate commits.

Copy link

LCOV of commit 840bf07 during Continuous Integration #4004

Summary coverage rate:
  lines......: 65.5% (14023 of 21421 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Copy link
Collaborator

@JanCBrammer JanCBrammer left a comment

Choose a reason for hiding this comment

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

If this is a bug fix, please add a test that replicates (or at least approximates) the bug and demonstrates how this PR fixes it.


if (range == 'all') {
if (this.state.currentSearchSelection && elements[type + "s"].ids) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to be sure, the only behavioral change in this file's diff is the removal of this.state.currentSearchSelection from the guard clause? For the future please change behavior and formatting in separate commits.

@PiTrem PiTrem changed the base branch from main to v1.10 October 29, 2024 08:25
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.

3 participants