Page MenuHome

UI: Fixes for the close file dialog box layout
ClosedPublic

Authored by Yevgeny Makarov (jenkm) on Feb 26 2020, 3:15 PM.

Details

Summary

Fixes for the close file dialog box layout.

Fixed misalignment of buttons and checkboxes.

With fixed D9058 labels, and D8394 text changes
the dialog box will look this way:

Diff Detail

Repository
rB Blender

Event Timeline

Yevgeny Makarov (jenkm) retitled this revision from [WIP] New dialog box layout with a large alert icon. to UI: New dialog box layout with a large alert icon.
Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

Looks very good, but I'm unsure if it doesn't just overlap with D6859, which also seems to include your edits?

It's okay if this patch is merged with D6859 (that's what it was originally meant to be), if you think a single large/combined patch is better.
Or we could split D6859 so that D6859 and this patch can be applied simultaneously without conflicts.

Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

uncommented uiDefButAlert, so now this patch depends on D6859

Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

This is now a small change, but the adjustments here do seem a little nicer.

This revision is now accepted and ready to land.Mar 17 2020, 11:20 AM

updated to reflect current state of master

Yevgeny Makarov (jenkm) updated this revision to Diff 23970.EditedApr 22 2020, 2:48 PM
Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

Need some corrections after changes in the checkboxes.

Yevgeny Makarov (jenkm) retitled this revision from UI: New dialog box layout with a large alert icon to UI: Fixes for the close file dialog box layout.
Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

updated for 2.91

Hans Goudey (HooglyBoogly) requested changes to this revision.Oct 5 2020, 5:36 PM

Definitely looks better. I think having some comments describing the decisions here wouldn't be a bad idea though. They can seem a bit arbitrary otherwise.

And just for the git history sometimes it's nice to at least have a very brief description in the commit / diff of why it didn't work before and why it works now.

source/blender/windowmanager/intern/wm_files.c
3150

It would be nice to have a comment with some information as to why "6" specifically makes sense here.

Also, U.dpi_fac is already a float, and so is 6.0f, so it's only "icon_size" that needs a cast: ((float)icon_size + 6.0f * U.dpi_fac))

3256

Seems weird to have a split with a factor of zero, would a normal row work here?

This revision now requires changes to proceed.Oct 5 2020, 5:36 PM

It would be nice to have a comment with some information as to why "6" specifically makes sense here.

Yes, it's mysterious. It's hard-coded 14 - 8.

This makes the margin between the icon and the text equal to the margin on the left, from the edge of the block and the icon.

UI_block_bounds_set_centered(block, 14 * U.dpi_fac)
style->columnspace = 8


Seems weird to have a split with a factor of zero, would a normal row work here?

uiLayoutSplit(layout, 0.0f, true)

The 0.0f means equal columns; also, this was used here until the last changes.

I tried Row a few times but it didn't work here for some reason.


/* Alert Icon. */
uiLayout *layout = uiLayoutRow(split_block, false);
uiLayoutSetAlignment(layout, UI_LAYOUT_ALIGN_LEFT);
uiDefButAlert(block, ALERT_ICON_WARNING, 0, 0, icon_size, icon_size);

Here "Row" and "UI_LAYOUT_ALIGN_LEFT" to avoid the icon stretching along the full width of the column.

It would be nice to have a comment with some information as to why "6" specifically makes sense here.

Although in this case it was calculated as described in the comment above,
but in fact it is just so that the text is not too close to the icon,
that is, I don't think it's necessary to strictly calculate the variable depending on others,
just some extra text padding from the icon.

Yevgeny Makarov (jenkm) marked an inline comment as done.

Thanks for adding the comments.

This revision is now accepted and ready to land.Wed, Oct 28, 5:47 PM

@Harley Acheson (harley) Here's another one to commit if you have the chance. : )

This revision was automatically updated to reflect the committed changes.