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

Add intro blurb and adjust detail page layout #110

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

brianlove
Copy link
Contributor

Include the Wikipedia intro blurb text (where available) and adjust the detail page layout to work better on smaller viewports.

@brianlove brianlove self-assigned this Sep 14, 2023
@github-actions
Copy link

github-actions bot commented Sep 14, 2023

No need for rebasing 👍
behind_count is 0
ahead_count is 2

@github-actions
Copy link

github-actions bot commented Sep 14, 2023

JavaScript Coverage

Summary

Lines Statements Branches Functions
Coverage: 73%
73.07% (266/364) 61.33% (92/150) 77.04% (94/122)
Modified Files • (73%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files73.0761.3377.0473.56 
components74.565.1177.8974.91 
   DetailView.jsx000028–150
   DetailViewIntro.jsx00006–61

Copy link
Contributor

@niharikasingh niharikasingh left a comment

Choose a reason for hiding this comment

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

I'm not sure to what extent you intended to address responsiveness in this ticket. I noticed a couple weird things for narrow viewports, but if you want to make a different ticket for them, you can merge this.

  1. I started with a wide window and then dragged it to be narrower. At 485px, there was overlap between text and graphs.

Screenshot 2023-09-15 at 11 19 48 AM

  1. I refreshed the screen on this narrow viewport, and while the overlap went away, the legend for the last chart is cut off.

Screenshot 2023-09-15 at 11 20 39 AM

Copy link
Member

@jmelot jmelot left a comment

Choose a reason for hiding this comment

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

This seems fine. The graph overlap issue Neha mentioned has come up before, if you don't want to address it here you could open an issue about it.

Can you call back to something like "No description available" for companies that don't have one (e.g. 1791-state-grid)? We have student annotators tracking these down so hopefully once they're done this won't be an issue, but still good to have the fallback

@brianlove brianlove force-pushed the detail-layout-and-content branch from 07c67b0 to b1f9b72 Compare September 25, 2023 14:35
@brianlove brianlove merged commit 2d7e71f into version2 Sep 25, 2023
@brianlove brianlove deleted the detail-layout-and-content branch September 25, 2023 14:46
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.

3 participants