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

FRONTEND: Update Screenshot functionality #2279

Merged

Conversation

eriwarr
Copy link
Collaborator

@eriwarr eriwarr commented Jul 12, 2023

Description

This PR adds the HET logo-text and removes unnecessary elements when taking screenshots.

  • Adds centered divider line, logo text, and exploredata/ url with params to the bottom of the screenshot using the html2Canvas library
  • url is shrunk to fit within the width of the screenshot
  • Adds elementsToHide prop to CardWrapper to hide a list of elements when capturing the chart image.
  • The dropdown menus are visible when open and removed when closed.
  • Adds unique element IDs for compare mode screenshots.

Motivation and Context

Has this been tested? How?

Tested locally

Screenshots (if appropriate):

COPD in the United States  from Health Equity Tracker Jul 2023 (1)
COPD in the United States  from Health Equity Tracker Jul 2023
Preventable hospitalizations in California for Asian, Native Hawaiian, and Pacific Islander (NH) from Health Equity Tracker Jul 2023 (13)

Types of changes

  • Bug fix
  • New content or feature

Post-merge TODO

I have inspected frontend changes and run affected data pipelines:

  • on DEV
  • on PROD

Any target user persona(s)?

Preview link below in Netlify comment 😎

@netlify
Copy link

netlify bot commented Jul 12, 2023

Deploy Preview for health-equity-tracker ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 03556da
🔍 Latest deploy log https://app.netlify.com/sites/health-equity-tracker/deploys/64c17020615f13000833f144
😎 Deploy Preview https://deploy-preview-2279--health-equity-tracker.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@eriwarr eriwarr requested a review from benhammondmusic July 17, 2023 14:28
@eriwarr eriwarr self-assigned this Jul 17, 2023
@eriwarr eriwarr requested a review from kccrtv July 17, 2023 14:30
@kccrtv
Copy link
Collaborator

kccrtv commented Jul 17, 2023

for the citation, if we're keeping in line with APA standards for electronic images, the format would generally be

Source. (Year of origin). Title of image [Online image]. Website name. URL

in the above screenshot, for example, the citation would be formatted as

CDC NCHHSTP AtlasPlus. (2019). HIV prevalence over time in the United States from Health Equity Tracker Jul 2023 [Online image]. Health Equity Tracker. https://healthequitytracker.org/

do you think we could alter the citation to match these standards?

@benhammondmusic
Copy link
Collaborator

URL looks good at the bottom. I feel like the logo placement is a little strange still... sorry to nitpick but im just thinking if the goal is to have people "save as" and then embed, we want it to look as good as the charts do on the page. Is it a pain to try and put the logo+logo text on the very bottom, centered? Or @kccrtv do you have any suggestions on making the export look as good as possible? If we need to drop the logo we can; i thought it'd be cool but the primary goal is to make it lok good enough someone wants to share it

Also @eriwarr , you noted that this PR closes #2186 , is that true? I thought you weren't able to get that fixed?

@eriwarr
Copy link
Collaborator Author

eriwarr commented Jul 19, 2023

URL looks good at the bottom. I feel like the logo placement is a little strange still... sorry to nitpick but im just thinking if the goal is to have people "save as" and then embed, we want it to look as good as the charts do on the page. Is it a pain to try and put the logo+logo text on the very bottom, centered? Or @kccrtv do you have any suggestions on making the export look as good as possible? If we need to drop the logo we can; i thought it'd be cool but the primary goal is to make it lok good enough someone wants to share it

Also @eriwarr , you noted that this PR closes #2186 , is that true? I thought you weren't able to get that fixed?

@benhammondmusic - I understand. No, centering the logo at the bottom wouldn't be difficult. Regarding #2186, I'm not sure, but I think my docking station/dell monitors are causing the grey issue. When I disconnected and downloaded the images as normal on my Mac, I no longer had the grey overlay problem. But we can leave it open as there is no way for me to confirm what else it could be.

@kccrtv
Copy link
Collaborator

kccrtv commented Jul 19, 2023

URL looks good at the bottom. I feel like the logo placement is a little strange still... sorry to nitpick but im just thinking if the goal is to have people "save as" and then embed, we want it to look as good as the charts do on the page. Is it a pain to try and put the logo+logo text on the very bottom, centered? Or @kccrtv do you have any suggestions on making the export look as good as possible? If we need to drop the logo we can; i thought it'd be cool but the primary goal is to make it lok good enough someone wants to share it
Also @eriwarr , you noted that this PR closes #2186 , is that true? I thought you weren't able to get that fixed?

@benhammondmusic - I understand. No, centering the logo at the bottom wouldn't be difficult. Regarding #2186, I'm not sure, but I think my docking station/dell monitors are causing the grey issue. When I disconnected and downloaded the images as normal on my Mac, I no longer had the grey overlay problem. But we can leave it open as there is no way for me to confirm what else it could be.

as we discussed earlier today, i like the idea of moving the H Health Equity Tracker logo to the very bottom, either centered or to the right. if there's a way to absolute fix it with a negative z-index and increased opacity, that could balance it out.

@eriwarr
Copy link
Collaborator Author

eriwarr commented Jul 20, 2023

URL looks good at the bottom. I feel like the logo placement is a little strange still... sorry to nitpick but im just thinking if the goal is to have people "save as" and then embed, we want it to look as good as the charts do on the page. Is it a pain to try and put the logo+logo text on the very bottom, centered? Or @kccrtv do you have any suggestions on making the export look as good as possible? If we need to drop the logo we can; i thought it'd be cool but the primary goal is to make it lok good enough someone wants to share it
Also @eriwarr , you noted that this PR closes #2186 , is that true? I thought you weren't able to get that fixed?

@benhammondmusic - I understand. No, centering the logo at the bottom wouldn't be difficult. Regarding #2186, I'm not sure, but I think my docking station/dell monitors are causing the grey issue. When I disconnected and downloaded the images as normal on my Mac, I no longer had the grey overlay problem. But we can leave it open as there is no way for me to confirm what else it could be.

as we discussed earlier today, i like the idea of moving the H Health Equity Tracker logo to the very bottom, either centered or to the right. if there's a way to absolute fix it with a negative z-index and increased opacity, that could balance it out.

how do you feel about these?
HIV prevalence  in  the United States  from Health Equity Tracker Jul 2023-20

HIV prevalence  in  the United States  from Health Equity Tracker Jul 2023-19

I can make whatever small changes to spacing/padding.

@kccrtv
Copy link
Collaborator

kccrtv commented Jul 20, 2023

URL looks good at the bottom. I feel like the logo placement is a little strange still... sorry to nitpick but im just thinking if the goal is to have people "save as" and then embed, we want it to look as good as the charts do on the page. Is it a pain to try and put the logo+logo text on the very bottom, centered? Or @kccrtv do you have any suggestions on making the export look as good as possible? If we need to drop the logo we can; i thought it'd be cool but the primary goal is to make it lok good enough someone wants to share it
Also @eriwarr , you noted that this PR closes #2186 , is that true? I thought you weren't able to get that fixed?

@benhammondmusic - I understand. No, centering the logo at the bottom wouldn't be difficult. Regarding #2186, I'm not sure, but I think my docking station/dell monitors are causing the grey issue. When I disconnected and downloaded the images as normal on my Mac, I no longer had the grey overlay problem. But we can leave it open as there is no way for me to confirm what else it could be.

as we discussed earlier today, i like the idea of moving the H Health Equity Tracker logo to the very bottom, either centered or to the right. if there's a way to absolute fix it with a negative z-index and increased opacity, that could balance it out.

how do you feel about these? HIV prevalence in the United States from Health Equity Tracker Jul 2023-20

HIV prevalence in the United States from Health Equity Tracker Jul 2023-19

I can make whatever small changes to spacing/padding.

let's just take out the H logo entirely 😬 and leave Health Equity Tracker. i feel like it should be an either/or situation.

@eriwarr eriwarr closed this Jul 20, 2023
@eriwarr eriwarr reopened this Jul 20, 2023
@benhammondmusic
Copy link
Collaborator

benhammondmusic commented Jul 26, 2023

Issue with extremely long URLs; they don't line wrap and the interfere with the watermark style right-aligned logo

http://localhost:3000/exploredata?mls=1.cardiovascular_diseases-3.preventable_hospitalizations-5.06&mlp=comparevars&group1=Black.NH&group2=A%7ENHPI.NH
Screen Shot 2023-07-26 at 10 50 55 AM

@benhammondmusic benhammondmusic added feature request New feature or request cleanup Code health cleanups frontend labels Jul 26, 2023
Copy link
Collaborator

@kccrtv kccrtv left a comment

Choose a reason for hiding this comment

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

👍🏼

@benhammondmusic benhammondmusic merged commit 764b42d into SatcherInstitute:main Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code health cleanups feature request New feature or request frontend
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants