Skip to content

Firefly-1469: Update DCE to support rubin needs #1742

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

Merged
merged 1 commit into from
Apr 18, 2025
Merged

Conversation

robyww
Copy link
Contributor

@robyww robyww commented Apr 15, 2025

Firefly-1469: Update DCE to support rubin needs

  • support sia wavelength
  • support dataServiceId
  • clean up
  • support sia obscore options using ObsCore panel
  • improve UI layout
  • refactor some of DynComponents
  • included ServiceDescriptorPanel in the improvements

Testing

  • https://rc-2025-3.irsakudev.ipac.caltech.edu/suit/
  • get the xml file below
  • upload it into the SUIT portal
  • The Collection Search tab will open
    • this is an DCE testing tab
    • this is only good for one search, to go back you will have to go the the upload panel first
  • click on a target in the HiPS
  • choose some wavelength bands
  • do the search

more-sia.xml.zip

@robyww robyww self-assigned this Apr 15, 2025
@robyww robyww added this to the 2025.3 milestone Apr 15, 2025
@robyww robyww requested a review from jaladh-singhal April 15, 2025 23:06
@robyww robyww marked this pull request as ready for review April 16, 2025 03:26
@robyww robyww force-pushed the FIREFLY-1469-dce-rubin branch from fc44c82 to 66db591 Compare April 16, 2025 18:41
Copy link
Member

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good but I found a couple of issues with the UI:

  1. The data products doesn't load for any row in the search results of collection search
  2. When I choose "Load as Table" radio option in upload panel, the resulting table's 1st row's data product has embedded search panel - is this expected? If yes, the styling needs to be fixed, the fields are truncated because of limited real estate.
  3. The Collection Search's layout styles can be improved:
    1. Search radius looks odd because it's taking full width
    2. em_ops field looks like it's part of Spectral coverage panel when expanded but it's not. It can use some vertical spacing to create visual separation or move it on top of Spectral coverage panel.
    3. Can we make Cone/Polygon part of the spatial search panel - see how LVF obscore search form in spherex does it? (I'm not sure if it's feasible here but the idea is UI looks much cleaner because everything is in their own collapsible)

{childComponents && childComponents}
</Stack>)
: undefined; //to avoid rendering empty div (that takes spacing in Stack)
{childComponents && childComponents}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{childComponents && childComponents}
{childComponents}

If it's undefined or anything not of node type, react won't render it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix it.

@robyww
Copy link
Contributor Author

robyww commented Apr 17, 2025

The data products doesn't load for any row in the search results of collection search

I meant to mention that the URLs were broken for SIA searches. This is a rubin backend issue

When I choose "Load as Table" radio option in upload panel, the resulting table's 1st row's data product has embedded search panel - is this expected?

Yes, it is part of the general service descriptor handling which almost never happens in normal use cases. A end user is not going to upload a datalink file either. It is building a UI from the service descriptor.

@robyww robyww force-pushed the FIREFLY-1469-dce-rubin branch from 03a5d6c to 1b26aee Compare April 17, 2025 21:30
@robyww
Copy link
Contributor Author

robyww commented Apr 17, 2025

em_ops field looks like it's part of Spectral coverage panel when expanded but it's not. It can use some vertical spacing to create visual separation or move it on top of Spectral coverage panel.

That was just a test field, just to proved it worked. I meant to remove it frem the test.

@robyww
Copy link
Contributor Author

robyww commented Apr 17, 2025

Search radius looks odd because it's taking full width

I will clean that up.

Can we make Cone/Polygon part of the spatial search panel - see how LVF obscore search form in spherex does it? (I'm not sure if it's feasible here but the idea is UI looks much cleaner because everything is in their own collapsible)

I don't think it is a good idea in the context. It is always required and can't be turned off. I am trying to clean up the layout.

Copy link
Member

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Search radius looks odd because it's taking full width

I will clean that up.

Can we make Cone/Polygon part of the spatial search panel - see how LVF obscore search form in spherex does it? (I'm not sure if it's feasible here but the idea is UI looks much cleaner because everything is in their own collapsible)

I don't think it is a good idea in the context. It is always required and can't be turned off. I am trying to clean up the layout.

@robyww thanks - it looks more clear now with centered layout that this spatial fields are constant and rest of the fields below it are additional options.

We can iterate more over the design as when we work on improving Embedded search position panel's layout in future.

 - support sia wavelength
 - support sia obscore options using ObsCore panel
 - support dataServiceId
 - clean up
 - improve UI layout
 - refactor some of DynComponents
 - included ServiceDescriptorPanel in the improvements
 - includes response to feedback
@robyww robyww force-pushed the FIREFLY-1469-dce-rubin branch from 1b26aee to 57244d3 Compare April 18, 2025 18:25
@robyww robyww merged commit 8d14855 into dev Apr 18, 2025
@robyww robyww deleted the FIREFLY-1469-dce-rubin branch April 18, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants