Skip to content

Configure the logo rather than overriding#1563

Merged
eliotjordan merged 1 commit intomainfrom
configure-logo
Aug 23, 2024
Merged

Configure the logo rather than overriding#1563
eliotjordan merged 1 commit intomainfrom
configure-logo

Conversation

@jcoyne
Copy link
Collaborator

@jcoyne jcoyne commented Aug 23, 2024

This makes it easier for downstream apps to configure their own logos and results in less overall css.

@github-actions
Copy link

github-actions bot commented Aug 23, 2024

Demo app download link: https://github.com/geoblacklight/geoblacklight/actions/runs/10531276655/artifacts/1848614946

  1. Download demo app and unzip file
  2. Change into app directory
    • run docker compose pull
    • run docker compose up
  3. Open in browser

@jcoyne jcoyne force-pushed the configure-logo branch 4 times, most recently from e58fc1c to 9197696 Compare August 23, 2024 16:04
@eliotjordan eliotjordan force-pushed the configure-logo branch 3 times, most recently from c09dfc6 to 0d29dca Compare August 23, 2024 16:53
Copy link
Member

@eliotjordan eliotjordan left a comment

Choose a reason for hiding this comment

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

Though it seems like this should work, the generated app has a blank logo.
Screenshot 2024-08-23 at 11 27 27 AM

@jcoyne
Copy link
Collaborator Author

jcoyne commented Aug 23, 2024

Screenshot 2024-08-23 at 12 02 48 PM

@jcoyne
Copy link
Collaborator Author

jcoyne commented Aug 23, 2024

This has got to be broken on the main branch too, right? Because image_url is a sprockets thing and we're using propshaft?

@jcoyne
Copy link
Collaborator Author

jcoyne commented Aug 23, 2024

Nope, in main:
Screenshot 2024-08-23 at 12 07 39 PM

@jcoyne jcoyne force-pushed the configure-logo branch 3 times, most recently from 002cd46 to 26d3979 Compare August 23, 2024 19:19
@jcoyne
Copy link
Collaborator Author

jcoyne commented Aug 23, 2024

@eliotjordan I had to update the vite config for the so that It can deal with the yarn link that we do in the test app generator

@eliotjordan eliotjordan merged commit 6590131 into main Aug 23, 2024
@eliotjordan eliotjordan deleted the configure-logo branch August 23, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants