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

Radio links #50

Merged
merged 8 commits into from Mar 25, 2023
Merged

Radio links #50

merged 8 commits into from Mar 25, 2023

Conversation

mgeraci
Copy link
Collaborator

@mgeraci mgeraci commented Mar 25, 2023

This PR has some style tweaks in the radio section, including adding external links to the radio program permalink page.

List page

  • move "podcast feed" to the right side, adjust spacing
  • add space between paragraphs in the episode description
  • adjust vertical alignment with the audio element and download icon

Screenshot 2023-03-25 at 12 42 42 PM

Detail page

  • add the podcast feed and apple link
  • update the "original air date" copy's styles
  • add space between paragraphs in the episode description
  • adjust vertical alignment with the audio element and download icon

Screenshot 2023-03-25 at 12 42 59 PM

Other fixes

  • add an nvmrc file to help remember what version is required in the future

Comment on lines 186 to 188
.external-links
display: flex
align-items: center
Copy link
Owner

@katur katur Mar 25, 2023

Choose a reason for hiding this comment

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

Nit: I don't love that this is repeated, but don't mind waiting to fix. Or, we could move it to top-level (qualified as podcast-external-links), a la .audio-with-download

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I addressed this by extracting styles for that component to their own sheet. Good idea!

@@ -191,6 +213,13 @@ $mobile-breakpoint: 500px
@include markdown-image-wrapper(500px)


.audio-with-download
Copy link
Owner

Choose a reason for hiding this comment

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

This might affect "Sound Recordings" as well. If so, maybe it shouldn't live in the radio.sass (since it is included elsewhere)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked and this fix applied to that page as well 🤙

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We moved these styles to the elements sheet, which handles more globally-applicable element styles.

Copy link
Owner

@katur katur left a comment

Choose a reason for hiding this comment

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

:shipit: Thank you!!

@mgeraci mgeraci merged commit 37608ca into master Mar 25, 2023
@mgeraci mgeraci deleted the radio-links branch March 25, 2023 19:56
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.

None yet

2 participants