Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Inconsistent use of lib-react-component's Media component #1133

Closed
srallen opened this issue Sep 18, 2019 · 3 comments
Closed

Inconsistent use of lib-react-component's Media component #1133

srallen opened this issue Sep 18, 2019 · 3 comments
Labels
discussion Talk about how to do something refactor Refactoring existing code
Projects

Comments

@srallen
Copy link
Contributor

srallen commented Sep 18, 2019

Package app-project

Feature or Issue Description
Media is built to use the thumbnail service which is in parallel with what we did on PFE. The reason is that some project have uploaded and/or are using massively sized images in their Markdown. There's already some image use on the project app not using the Media component. We should discuss why that's the case, i.e. does the Media component need some modifications to make it applicable to these use cases? Or if it can't be used, maybe make a note of that and independently use the thumbnail service in these cases.

@srallen srallen added the discussion Talk about how to do something label Sep 18, 2019
@srallen srallen added this to To do in Project app via automation Sep 18, 2019
@srallen srallen mentioned this issue Sep 18, 2019
7 tasks
@rogerhutchings
Copy link
Contributor

rogerhutchings commented Sep 19, 2019

In #1077, I didn't use Media because:

  • the html it renders is a bit weird:

    <noscript>
      <div style="max-height:999px;max-width:999px">
        <img src="" alt="" height="100%" width="100%" style="flex:grow;object-fit:cover"/>
      </div>
    </noscript>
    

    This breaks without JS enabled.

  • it caused a big teal placeholder to appear before the image rendered which was quite jarring.

If the goal is to make sure we use the thumbnail service, I'd probably find a HOC that passes in a function prop that can change an img's src more useful - it could then be used a few times to build up a srcSet, for example.

@eatyourgreens
Copy link
Contributor

In PFE, the Thumbnail component includes a helper function to convert URLs. We use it to generate background images for styling, for example. Here's an example from the CroppedImage component, which renders an image cropped to the SVG viewbox, so can't render an img tag.

https://github.com/zooniverse/Panoptes-Front-End/blob/21cf42485929a62938112d5e2d4bca1a6702b00e/app/components/cropped-image.jsx#L26-L31

@eatyourgreens
Copy link
Contributor

Related to this: Media generates invalid CSS height and width #1147, leading to layout bugs.

@rogerhutchings rogerhutchings added the refactor Refactoring existing code label Nov 11, 2019
@zooniverse zooniverse locked and limited conversation to collaborators Aug 8, 2023
@goplayoutside3 goplayoutside3 converted this issue into discussion #5129 Aug 8, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
discussion Talk about how to do something refactor Refactoring existing code
Projects
Project app
  
To do
Development

No branches or pull requests

3 participants