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

Add data info line to SVG export dialog. #1192

Closed
wants to merge 2 commits into from

Conversation

lisham2000
Copy link
Contributor

No description provided.

Comment on lines 3301 to 3304
converter = CalibratedSizeFloatToStringConverter(self, index, uniform=False)
calibrated_dimensional_calibration_str_list.append(converter.convert_calibrated_value_to_str(display_data_shape[index]))
converter = CalibratedSizeFloatToStringConverter(self, index, uniform=True)
calibrated_value = converter.convert(1)
if calibrated_value is not None:
calibrated_dimensional_calibration_str_list.append(calibrated_value) #passing in 1 as the converter expects fractional units
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this being changed? This belongs in a separate PR with an explanation.

Also, the format of the comment would need to follow Python conventions with two spaces before the '#' and a space after the '#'.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See related PR #1214

@cmeyer
Copy link
Collaborator

cmeyer commented Oct 16, 2024

Repeating some info from chat here:

There is a problem if you calibrate an image in the x-dimension but not the y-dimension. This is a problem with the code we wrote the other day, so I'll provide a PR for it separately. PR here #1214.

The display of information needs to be improved.

image

We tend not to use colons so maybe the first line could be "Data shape (pixels)" and the 2nd line "Shape (calibrated units)"

And maybe to keep things uniform, the first line should have "Title" label.

Maybe put title, data shape, and calibrated shape (if displayed) into a grid where the labels are on the left and the values are on the right but the right column is left justified, basically like the inspector info section

image

(like this layout above, with labels on the left, values on the right)

The overall Export SVG window should be wider - try 480px. Then don't wrap the title field.

When using "\n" for formatting, make two text canvas items instead. "\n" isn't well defined in UI's.

After everything is in a table (use the same table layout mechanism used for width/height), you should also remove the colons from width/height/units.

Finally, the code itself in this PR is not following standard formatting. Using PyCharm, you should select the blocks of code that you're editing (but not anything else), for example lines 286-287 in the PR, and then choose Code > Reformat Code to make it look nice. It will add spaces around the + operators

You should more or less do this on all code that has been changed in the PR, but nothing more (no spurious white space changes).

Updated the SVG-1 and added an SVG-2 test for you test with.

svg-export-tests.zip

Copy link
Collaborator

@cmeyer cmeyer 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 getting better, but needs some obvious fixes. Also, can you rebase on the master and force push? This change should just be a single file ExportDialog.py.

image

The labels are wrong. For instance 2nd line is "Shape ... Data Shape (Pixels)" The label on the left should be "Data Shape (Pixels)". Likewise for the 3rd line which is inexplicably "Title" again.

Also, the text fields should not be the full width of the dialog. They should have a fixed width of about 100 pixels and then a stretch on the row so they don't expand. Same for the chooser menu item, but it doesn't need to have a fixed width; just a stretch after it.

@lisham2000 lisham2000 force-pushed the SVGInfoLabelNew branch 2 times, most recently from 3f9adad to ffbf125 Compare October 22, 2024 09:03
@cmeyer
Copy link
Collaborator

cmeyer commented Oct 25, 2024

Merged here c7916c0 with some additional tweaks here 57773e9 and d255482

@cmeyer cmeyer closed this Oct 25, 2024
@lisham2000 lisham2000 deleted the SVGInfoLabelNew branch October 29, 2024 14:56
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