Skip to content

Commit

Permalink
Merge pull request #4407 from gemini-hlsw/sc-4279-target-confusion
Browse files Browse the repository at this point in the history
  • Loading branch information
toddburnside authored Dec 20, 2024
2 parents 4c17817 + 9bd076a commit 9a17a65
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 20 deletions.
10 changes: 5 additions & 5 deletions explore/src/main/scala/explore/observationtree/ObsTree.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package explore.observationtree
import cats.effect.IO
import cats.syntax.all.*
import crystal.react.*
import crystal.react.hooks.*
import eu.timepit.refined.types.numeric.NonNegShort
import eu.timepit.refined.types.string.NonEmptyString
import explore.Icons
Expand Down Expand Up @@ -77,6 +76,7 @@ case class ObsTree(
pasteCallback: Callback,
clipboardObsContents: Option[ObsIdSet],
allocatedScienceBands: SortedSet[ScienceBand],
addingObservation: View[AddingObservation],
readonly: Boolean
) extends ReactFnProps(ObsTree.component):
private val selectedObsIdSet: Option[ObsIdSet] =
Expand Down Expand Up @@ -206,18 +206,18 @@ object ObsTree:
group => focusGroup(props.programId, group.id.some, ctx)
)
case _ => Callback.empty
.useStateView(AddingObservation(false)) // adding new observation
// Scroll to newly created/selected observation
.useEffectWithDepsBy((props, _, _, _) => props.focusedObs): (_, _, _, _) =>
.useEffectWithDepsBy((props, _, _) => props.focusedObs): (_, _, _) =>
focusedObs => focusedObs.map(scrollIfNeeded).getOrEmpty
// Open the group (and all super-groups) of the focused observation
.useEffectWithDepsBy((props, _, _, _) => props.activeGroup): (props, _, _, _) =>
.useEffectWithDepsBy((props, _, _) => props.activeGroup): (props, _, _) =>
_.map: activeGroupId =>
props.expandedGroups.mod(_ ++ props.parentGroups(activeGroupId.asRight) + activeGroupId)
.orEmpty
.render: (props, ctx, _, adding) =>
.render: (props, ctx, _) =>
import ctx.given

val adding: View[AddingObservation] = props.addingObservation
val expandedGroups: View[Set[Tree.Id]] = props.expandedGroups.as(groupTreeIdLens)

def onDragDrop(e: Tree.DragDropEvent[Either[Observation, Group]]): Callback =
Expand Down
7 changes: 5 additions & 2 deletions explore/src/main/scala/explore/tabs/ObsTabContents.scala
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ object ObsTabContents extends TwoPanels:
callbacks
)
.useStateView(DeckShown.Shown)
.useStateView(AddingObservation(false)) // adding new observation or duplicating
.render:
(
props,
Expand All @@ -188,7 +189,8 @@ object ObsTabContents extends TwoPanels:
selectedOrFocusedObsIds, // Mixes focused obs and selected obs in table
copyCallback,
pasteCallback,
deckShown
deckShown,
addingObservation
) =>
val observationsTree: VdomNode =
if (deckShown.get === DeckShown.Shown) {
Expand All @@ -211,6 +213,7 @@ object ObsTabContents extends TwoPanels:
pasteCallback,
shadowClipboardObs.value,
props.programSummaries.get.allocatedScienceBands,
addingObservation,
props.readonly
)
} else
Expand Down Expand Up @@ -312,7 +315,7 @@ object ObsTabContents extends TwoPanels:
props.userPreferences.get.observationsTabLayout,
resize,
props.globalPreferences,
props.readonly
props.readonly || addingObservation.get.value
).withKey(s"${obsId.show}")
)
}
Expand Down
54 changes: 42 additions & 12 deletions explore/src/main/scala/explore/targeteditor/SearchForm.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,17 @@ import lucuma.ui.reusability.given
import lucuma.ui.syntax.all.given
import org.scalajs.dom

import scala.concurrent.duration.*

import scalajs.js.JSConverters.*

case class SearchForm(
id: Target.Id,
targetName: View[NonEmptyString],
targetSet: Target.Sidereal => Callback,
searching: View[Set[Target.Id]],
readonly: Boolean
id: Target.Id,
targetName: View[NonEmptyString],
targetSet: Target.Sidereal => Callback,
searching: View[Set[Target.Id]],
readonly: Boolean,
cloningTarget: Boolean
) extends ReactFnProps[SearchForm](SearchForm.component)

object SearchForm:
Expand Down Expand Up @@ -65,17 +68,36 @@ object SearchForm:
)
.when_(props.targetName.get === NewTargetName)
)
.render: (props, ctx, term, enabled, error, buttonRef, inputRef) =>
.useSingleEffect
.render: (props, ctx, term, enabled, error, buttonRef, inputRef, singleEffect) =>
import ctx.given

val searchComplete: Callback = props.searching.mod(_ - props.id)

def onKeyPress = (e: ReactKeyboardEvent) =>
if (Option(e.key).exists(_ === dom.KeyValue.Enter) && !props.readonly)
// we just click the button, which deals with the name update/cloning/delay
// as details in comments below.
buttonRef.get >>= (_.map(button => Callback(button.click())).orEmpty)
else
Callback.empty

def onButtonClick: Callback =
// This will cancel the name update, as described in a comment below.
singleEffect
.submit(
(error.setState(none) >> props.searching.mod(_ + props.id)).toAsync
)
.runAsync

// if the user cancels the search, the term (in the text input) might not
// match the target name, so we need to update the target name to match.
// This is the behavior requested by Andy.
def updateNameIfNeeded: Callback =
if (term.get =!= props.targetName.get)
props.targetName.set(term.get)
else Callback.empty

val searchIcon: VdomNode =
if (enabled.value && !props.readonly)
TargetSelectionPopup(
Expand All @@ -87,29 +109,37 @@ object SearchForm:
Icons.ArrowDownLeft,
trigger = Button(
severity = Button.Severity.Success,
disabled = props.readonly || props.searching.get.nonEmpty,
disabled = props.cloningTarget || props.searching.get.nonEmpty,
icon = Icons.Search,
loading = props.searching.get.nonEmpty,
onClick = error.setState(none) >> props.searching.mod(_ + props.id),
onClick = onButtonClick,
modifiers = List(^.untypedRef := buttonRef)
).tiny.compact,
onSelected = targetWithId =>
(targetWithId.target match
case t @ Target.Sidereal(_, _, _, _) => props.targetSet(t)
case _ => Callback.empty
) >> searchComplete,
onCancel = searchComplete,
onCancel = updateNameIfNeeded >> searchComplete,
initialSearch = term.get.some,
showCreateEmpty = false
)
else Icons.Ban

val disabled = props.searching.get.exists(_ === props.id) || props.readonly
val disabled =
props.searching.get.exists(_ === props.id) || props.readonly || props.cloningTarget

// use a form here to handle submit? is it embedded?
FormInputTextView(
id = "search".refined,
value = term.withOnMod(props.targetName.set),
value = term.withOnMod(nes =>
// We submit to the singleEffect with a delay. If the user hit Enter or they
// click on the button before leaving the input field, this will get cancelled
// so that the name doesn't get updated and, more importantly, no target cloning
// occurs, which messed everything up.
singleEffect
.submit(IO.sleep(200.milliseconds) >> props.targetName.set(nes).toAsync)
.runAsync
),
label = React.Fragment("Name", HelpIcon("target/main/search-target.md".refined)),
validFormat = InputValidSplitEpi.nonEmptyString,
error = error.value.orUndefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ object SiderealTargetEditor:
nameView,
allView.set,
props.searching,
props.readonly || props.obsInfo.isReadonly
props.readonly || props.obsInfo.isReadonly,
cloning.get
),
FormInputTextView(
id = "ra".refined,
Expand Down

0 comments on commit 9a17a65

Please sign in to comment.