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

1791 Mugglify CIF Importer #1799

Merged
merged 25 commits into from
Feb 21, 2024
Merged

1791 Mugglify CIF Importer #1799

merged 25 commits into from
Feb 21, 2024

Conversation

trisyoungs
Copy link
Member

@trisyoungs trisyoungs commented Feb 16, 2024

This PR performs a significant refactoring on the CIFImportDialog, turning it from a wizard into a fully-featured single-page dialog. Aside from, IMHO, a better UX, a significant secondary reason was the desire to streamline the underlying CIF generation and move away from doing the full, multi-stage generation every time a parameter was changed.

The dialog itself now looks like this, with a toolbox on the left-hand side containing all the tweakable parameters, but the majority of space given over to actually seeing what the generated structure will be:
image

In other news, we also remove the local CoreData object and just go for local instances of the necessary (temporary) objects. A few bug fixes also along the way. Lovely.

Closes #1791.

@trisyoungs trisyoungs force-pushed the 1791-mugglify-cif-importer branch from 7e4aca5 to 290730f Compare February 16, 2024 15:57
@jws-1
Copy link
Member

jws-1 commented Feb 18, 2024

I see this is not ready for review yet, but just wanted to chime in and say I like the new look of things! A single page dialog is definitely the way to go (I got very fed up of the wizard when I was working on this stuff before!).

@trisyoungs
Copy link
Member Author

Yeah, I think my view on wizards has changed to the polar opposite now - can't stand em! Particularly in the case of the CIF import, it felt like didn't give the users or the developers the best view on things.

Base automatically changed from 1690-molecularize-cif to develop February 19, 2024 08:58
@trisyoungs trisyoungs force-pushed the 1791-mugglify-cif-importer branch from 290730f to f81da8e Compare February 19, 2024 09:03
@trisyoungs trisyoungs added this to the Release 1.5 milestone Feb 19, 2024
@trisyoungs trisyoungs marked this pull request as ready for review February 19, 2024 09:32
@trisyoungs trisyoungs force-pushed the 1791-mugglify-cif-importer branch from f81da8e to 8bd65bd Compare February 19, 2024 10:29
src/gui/menu_species.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@rhinoella rhinoella 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, minor comments.

src/gui/importCIFDialog.cpp Show resolved Hide resolved
src/gui/importCIFDialog.cpp Outdated Show resolved Hide resolved
src/gui/importCIFDialog.cpp Outdated Show resolved Hide resolved
src/gui/importCIFDialog.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@rhinoella rhinoella 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!

@trisyoungs trisyoungs merged commit 55bf0d4 into develop Feb 21, 2024
10 checks passed
@trisyoungs trisyoungs deleted the 1791-mugglify-cif-importer branch February 21, 2024 09:57
rprospero pushed a commit that referenced this pull request Mar 11, 2024
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.

Mugglify the CIF Importer
3 participants