Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
8313424: JavaFX controls in the title bar #1605
base: master
Are you sure you want to change the base?
8313424: JavaFX controls in the title bar #1605
Changes from all commits
3d19b87
0ddd63d
7969622
a289572
ba02e8f
f973e8c
f02e7e9
fef8cfc
778e6c1
3b468fe
0526edb
95736df
d9c0fe2
c0b588f
d7f88c3
9de4694
cd5d443
bc48ae0
804d0be
f5e3121
1c4ecc1
15dc3ff
9b63892
e7febc5
d1c388b
8c9fbbd
3660a29
8974c14
ca9b325
8e77a22
4336735
a9178b7
6a16536
65f095e
d5afc7d
26b81b8
003e9d5
485a9d9
743626f
d9b82c8
8649f91
af35dce
6523073
8d5d7b8
eaafd9f
cbb3216
63bd058
8c33b3c
49a81d3
356a78e
fcec7c8
2ab1294
8a73f5e
e874d21
3efd782
cadf1c2
600c721
7cb591f
c30eb60
94ce7d4
a04d946
00b9be7
9a7246b
1f95e9d
cd9c2a9
394c95f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing newline after L714?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, doesn't look nice, but I wanted to match the style of the surrounding methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make more sense to use Insets instead of Dimension2D? What if the app calls for asymmetrical padding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is not API, so it doesn't matter here. However, it matters in
HeaderBar.leftSystemInset/rightSystemInset
, which are both of typeDimension2D
. The system insets are not arbitrary rectangles, they are always aligned with the top-left and top-right edges of the window. As such, the only necessary information is their spatial extent, which is best represented as aDimension2D
.I'm not sure what you mean by asymmetrical padding. Can you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word "inset" confused me. Could you use some other word here, if it is referring to a dimension? Like maybe explain where exactly these are used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we talking about the specification of the system-reserved insets in
HeaderBar
? Or do you suggest to link fromHeaderButtonMetrics
toHeaderBar
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a link would be nice, yes (for javafx developers) :-)
it could be just me - the word "inset" confused me ("insets" ->
Insets
, "inset" ->Dimension2D
), but I guess it does describe the concept correctly.