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

Define distributions on the UI #2738

Merged
merged 2 commits into from
Jan 22, 2024
Merged

Conversation

smk78
Copy link
Contributor

@smk78 smk78 commented Jan 17, 2024

Description

This PR addresses a criticism aired in #2390 that Users seem confused by what type of average our polydispersity distributions represent, even though this is clearly stated in the docs and on our FAQ webpage. The specific request was that something be added to the UI.

The solution provided is shown here:
image

How Has This Been Tested?

Tested as a developer build on W10/x64.

Review Checklist (please remove items if they don't apply):

  • Code has been reviewed
  • Functionality has been tested
  • Windows installer (GH artifact) has been tested (installed and worked)
  • MacOSX installer (GH artifact) has been tested (installed and worked)

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

This is technically correct, however...
You are overwriting the existing title which is defined in FittingWidgetUI.ui.
It would probably be better to modify the title in the UI file instead.

@gonzalezma
Copy link
Contributor

I did the change proposed by Piotr. I think that this can be merged, but now someone else should review it.

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

looks good now!

@rozyczko rozyczko merged commit 8ea6865 into main Jan 22, 2024
18 checks passed
@rozyczko rozyczko deleted the 2390-define-polydispersity-parameters branch January 22, 2024 14:53
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