Details
- Reviewers
William Reynish (billreynish) Julian Eisel (Severin) Hans Goudey (HooglyBoogly) - Group Reviewers
User Interface - Commits
- rBb0a9a04f6201: UI: Improvements to Close File Dialog
Diff Detail
- Repository
- rB Blender
Event Timeline
Looks very good, but I'm unsure if it doesn't just overlap with D6859, which also seems to include your edits?
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? | |
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.


