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

feat: Allow setting of temperature through button & update docs #1855

Merged
merged 8 commits into from
Apr 16, 2024

Conversation

rhinoella
Copy link
Contributor

Removes depreciated TemperatureProcedureNode and instead sets temperature through a toolbutton in the configuration tab.

@rhinoella rhinoella mentioned this pull request Apr 15, 2024
src/gui/configurationTab.cpp Outdated Show resolved Hide resolved
src/procedure/nodes/CMakeLists.txt Outdated Show resolved Hide resolved
src/procedure/nodes/registry.cpp Outdated Show resolved Hide resolved
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

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

Sorry - sent you a message on Teams expressing how bad I got this wrong. Made way more work for you than it should have been!

I just tried the signal connection, and for me adding this in the ConfigurationTab constructor worked just fine, and lets you simplify your PR quite a bit:

connect(ui_.TemperatureToolButton, SIGNAL(clicked(bool)), dissolveWindow_, SLOT(on_ConfigurationAdjustTemperatureAction_triggered(bool)));

examples/argon/input.txt Outdated Show resolved Hide resolved
@rhinoella rhinoella changed the title feat: Remove TemperatureProcedureNode and set temperature through toolbutton feat: Allow setting of temperature through button & update docs Apr 16, 2024
@rhinoella
Copy link
Contributor Author

@trisyoungs Please double-check the changes to the Sillica example, I am not entirely sure if the node is appropriate for this example.

Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

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

Need a change to the silica example. Did you see my suggestion for connecting the signal as well?

web/docs/examples/silica/step5.md Outdated Show resolved Hide resolved
web/docs/examples/argon/step3.md Outdated Show resolved Hide resolved
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

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

👍

@rhinoella rhinoella merged commit 2ef3002 into develop Apr 16, 2024
11 checks passed
@rhinoella rhinoella deleted the temp-set branch April 16, 2024 17:24
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.

None yet

2 participants