Add captions back to avalanche forecast #1065
Conversation
| thumbnailAspectRatio = 1.3, | ||
| mediaItems, | ||
| displayCaptions = true, | ||
| displayCaptions = false, |
There was a problem hiding this comment.
why make the default false? is there a case in the media carousel when we dont show captions?
There was a problem hiding this comment.
I'll update the name so it's more clear. This is actually controlling if the captions are being displayed in the thumbnail list, not in the modal.
The use case for the AvalancheProblemCard is actually the only place where we show the caption with the thumbnail which is why I switched it to false. A potentially better fix is to maybe create a new MediaPreview component that would specifically handle this instead of making the ThumbnailList do it.
There was a problem hiding this comment.
The main reason being is that in the AvalancheProblemCard we're forcing the array that's passed into contain only 1 item, it's the only place where the captions are inline before opening up the modal to see the full image, and it's the only place where the thumbnail is a different size. A new component could make it more clear what we're trying to show, but this is the quicker fix
There was a problem hiding this comment.
make an issue for making a new component in the future, but the current quick fix makes sense (please add a comment with this context above the false default)
There was a problem hiding this comment.
Okay sounds good!
There was a problem hiding this comment.
2 participants
Added the captions back to the avalanche forecast. This was a bug before adding the new media carousel, but that new code accidentally removed it entirely. This brings it back