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

Only display outer lone pairs if "Show Outer Lone Pairs" and "Show Lone Pairs" are checked #194

Closed
Nancy-Salpepi opened this issue Oct 12, 2021 · 6 comments
Assignees

Comments

@Nancy-Salpepi
Copy link

The checkbox for 'show outer lone pairs' is currently in the Options dialog found in the PhET menu.
This means that this option is not affected by the Reset All button (and the location seems odd in general).
Perhaps move to Options Panel and couple with 'show loan pairs?'

In studio, I also don't see anywhere in the tree that would change the visible property of the 'Show Outer Lone Pairs' checkbox to hide it from students. All I see is:
Screen Shot 2021-10-12 at 2 53 31 PM

which just checks/unchecks the property.

@arouinfar
Copy link
Contributor

Thanks @Nancy-Salpepi!

Show Outer Lone Pairs is a more teacher-facing control. We want teachers to be able to opt-in to showing all lone pairs in the molecule, not just those on the central atom. This is not something we want students to naturally encounter, which is why it's in the Options menu.

That said, I do find it really strange that "Show Lone Pairs" only applies to the lone pairs on the central atom. I would expect it to apply to all lone pairs. If Show Outer Lone Pairs is checked, I want it to respect the state of the Show Lone Pairs checkbox in the sim.

@jonathanolson can change the behavior so that "Show Outer Lone Pairs" will display the outer lone pairs only when the "Show Lone Pairs" checkbox within the sim is checked?

In studio, I also don't see anywhere in the tree that would change the visible property of the 'Show Outer Lone Pairs' checkbox to hide it from students. All I see is:

@Nancy-Salpepi you can find the Options Dialog content in the tree under moleculeShapes.general.view.navigationBar.phetButton.phetMenu.optionsDialogCapsule.optionsDialog.content.

@arouinfar arouinfar assigned jonathanolson and unassigned arouinfar Oct 13, 2021
@arouinfar arouinfar changed the title Consider moving location of 'Show Outer Lone Pairs' checkbox Only display outer lone pairs if "Show Outer Lone Pairs" and "Show Lone Pairs" are checked Oct 13, 2021
@jonathanolson
Copy link
Contributor

Implemented above, @arouinfar can you verify?

@arouinfar
Copy link
Contributor

Thanks @jonathanolson it looks like all lone pairs respect the "Show Lone Pairs" checkbox within the sim. However, something strange is going on with the enabledProperty of the checkbox itself. The "Show Lone Pairs" checkbox is only enabled for molecules that have lone pairs on the central atom. This can lead to odd situations where there are lone pairs in the molecule, but the checkbox is disabled (such as CO2 or BF3).

In my opinion, it's not necessary to disable the checkbox for molecules that do not contain lone pairs. Can you instead leave "Show Lone Pairs" always enabled regardless of the selected molecule?

@jonathanolson
Copy link
Contributor

Changed above, can you verify?

@arouinfar
Copy link
Contributor

Looks good in master, thanks @jonathanolson. Not sure if you want this verified in the next QA cycle too or not, so back to you.

@arouinfar arouinfar assigned jonathanolson and unassigned arouinfar Nov 4, 2021
@jonathanolson
Copy link
Contributor

I'm fine closing, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants