-
Notifications
You must be signed in to change notification settings - Fork 69
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
Collapsible manœuvres #2664
Collapsible manœuvres #2664
Conversation
adapter_.plotting_frame_selector_.FrameParameters())) { | ||
frame_info = "Manœuvre frame differs from plotting frame"; | ||
info = "Manœuvre frame differs from plotting frame"; | ||
} | ||
UnityEngine.GUILayout.Label( |
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 placement of this string next to the Delete
button is weird, it seems to invite you to delete the manœuvre when the frames don't match. I would do something like:
Manœuvre #1 <grey>frame differs from plotting frame</grey>
That is, left align the message and shorten it a bit.
} | ||
UnityEngine.GUILayout.Label( | ||
frame_info, | ||
info, | ||
Style.RightAligned(Style.Info(UnityEngine.GUI.skin.label))); | ||
if (UnityEngine.GUILayout.Button("Delete", GUILayoutWidth(2))) { |
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.
It might look nicer if this button was next to +
/-
but I can see the point of not having Eject
and Format
next to each other.
Could we use 🗑
(U+1F5D1) as a label for this button? That would be clear enough.
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 font doesn’t support that.
@@ -56,29 +56,45 @@ class BurnEditor : ScalingRenderer { | |||
ComputeEngineCharacteristics(); | |||
} | |||
|
|||
public enum Event { | |||
None, |
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.
None
is mysterious. Unchanged
?
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.
It’s Event.None
, which makes some sense; Event.Unchanged
is weird.
bool anomalous, | ||
double burn_final_time, | ||
Action delete) { | ||
double burn_final_time) { | ||
bool changed = false; |
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 code would be a bit cleaner if you removed this bool
and had a local variable of type Event
.
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.
We need a bool
for the |=
. We should probably refactor this quite a bit, in particular so that it doesn’t return changed
on the first repaint, but not today.
initial_time, | ||
index, | ||
get_burn_at_index : burn_editors_.ElementAtOrDefault); | ||
editor.minimized = false; |
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.
Could we just say that a burn editor is created maximized? Shooting at the member variable is ugly.
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.
It is added maximized, but created minimized when loading an existing flight plan.
This is a property which is publicly write-only, not a member variable.
Once we add orbit analysis for coasts, the one-line coast descriptions will reflect that analysis (something like
Coast for 2 d 14 h in Earth orbit (5.2 rev.)
) and will be expandable to show:All manœuvres minimized:
![screenshot19](https://user-images.githubusercontent.com/2284290/89355770-86ae7f80-d6bc-11ea-8ee3-186f8c7c999d.png)
![screenshot20](https://user-images.githubusercontent.com/2284290/89355774-8910d980-d6bc-11ea-85fc-e5260ae04f38.png)
One expanded: