Skip to content
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

IRSA-6706, IRSA-6571: Add Image on background of Results tab #1724

Merged
merged 2 commits into from
Mar 7, 2025

Conversation

jaladh-singhal
Copy link
Member

@jaladh-singhal jaladh-singhal commented Mar 7, 2025

Fixes IRSA-6706, IRSA-6571

See ife PR: https://github.com/IPAC-SW/irsa-ife/pull/388

  • HydraLanding styles change if bgImage is supplied by consumers (ife apps) and changes in LandingPage to support that
  • Edit webpack config to support .webp files (SPHEREx image is available as webp) and to switch to asset/resource for large files (background images are large, we don't want them inline)
  • Several cleanups:
    • topSection now accepts title (to distinguish from appTitle), and desc that are used by Hydra apps
    • icon prop of HydraLanding is never used by topSection in any apps but bottomSection uses it, so moved it there to avoid confusion and make it easier for consumers to define it (without going through slotProps route)
    • Replaced setIfUndefined with defaultsDeep since there are lot of slotProps modification, and having single nested object is more readable when overriding

Testing

Check background image in landing page of:

Regression Testing in landing page of:

@jaladh-singhal jaladh-singhal added UI Client side UI changes not related to any of the visualizers multi-ticket This PR implements multiple Jira tickets labels Mar 7, 2025
@jaladh-singhal jaladh-singhal added this to the 2025.2 milestone Mar 7, 2025
@jaladh-singhal jaladh-singhal requested review from robyww and loitly March 7, 2025 17:54
@jaladh-singhal jaladh-singhal self-assigned this Mar 7, 2025
Copy link
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

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

Looks great.

@vandesai1
Copy link

Love seeing the images! Some comments/questions:

  1. The text for Euclid was changed in a different PR. That won't change things, right?
  2. Similarly, we will probably want to change the text for SPHEREx. We can do that later, right?
  3. Is there a possibility of having the spacecraft image to one side rather than directly behind the text box? (not sure if it would look better, but wondering if it's a possibility to consider)

@jaladh-singhal
Copy link
Member Author

Love seeing the images! Some comments/questions:

  1. The text for Euclid was changed in a different PR. That won't change things, right?
  2. Similarly, we will probably want to change the text for SPHEREx. We can do that later, right?
  3. Is there a possibility of having the spacecraft image to one side rather than directly behind the text box? (not sure if it would look better, but wondering if it's a possibility to consider)

@vandesai1 answers:

  1. yes (updated the build) - does the centre-aligned text look awkward, because we have too much text now?
  2. yes
  3. let me try - I have to crop image manually and then use it

@loitly
Copy link
Contributor

loitly commented Mar 7, 2025

  1. let me try - I have to crop image manually and then use it

@jaladh-singhal have you tried background-position-x/background-position-y? You may need to set min-width so the spacecraft does not wrap around.

@jaladh-singhal
Copy link
Member Author

  1. let me try - I have to crop image manually and then use it

@jaladh-singhal have you tried background-position-x/background-position-y? You may need to set min-width so the spacecraft does not wrap around.

@loitly I did, it doesn't work as intended because we're using background-size: cover which forces the image to fill the container always. We talked in FF-CCB to let ICE edit it and possibly also dim the background stars.

IRSA-6571: Add Euclid background image to landing page

IRSA-6571: Add Spherex background image to landing page

Edit webpack config to support .webp and to emit large images as files
IRSA-6571: Cleanup topSection logic in HydraApps

Move icon from topSection to bottomSection

IRSA-6571: Replace setIfUndefined with defaultsDeep for readability

IRSA-6571: Separate defaultProps logic into functions
@jaladh-singhal jaladh-singhal merged commit 4e08887 into dev Mar 7, 2025
@jaladh-singhal jaladh-singhal deleted the IRSA-6571-bg-img branch March 7, 2025 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multi-ticket This PR implements multiple Jira tickets UI Client side UI changes not related to any of the visualizers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants