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

Update style, add menu and change chart unit #26

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

barbara-chaves
Copy link
Collaborator

This PR

  • Updates the style
  • Adds menu with other tools links
  • Changes the ranking chart legend unit

Tasks

style: https://vizzuality.atlassian.net/browse/MP-176
ranking chart: https://vizzuality.atlassian.net/browse/MP-182

Copy link
Collaborator

@clementprdhomme clementprdhomme left a comment

Choose a reason for hiding this comment

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

Nice job Bárbara!

I'm adding three extra comments here:

  • The page has some vertical scroll. Do you think we could remove it?
  • The highlighted countries on the map are still green instead of purple
  • There are some issues with the exported image:
    • The Mongabay logo and attributions to Trase are gone
    • There's some extra padding at the top of the image

Before:
Image export with no blank space at the top, Mongabay's logo and attributions to Trase

After:
Image export with blank space at the top and bottom

components/header/menu/index.js Outdated Show resolved Hide resolved
components/header/menu/index.js Outdated Show resolved Hide resolved
components/header/menu/index.js Show resolved Hide resolved
components/tool/ranking/component.js Outdated Show resolved Hide resolved
components/tool/ranking/component.js Outdated Show resolved Hide resolved
components/header/menu/index.js Show resolved Hide resolved
Copy link
Collaborator

@clementprdhomme clementprdhomme left a comment

Choose a reason for hiding this comment

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

A few more comments:

  • I hadn't realised that you changed the order of the dropdowns before and I don't think it's a good idea. A change of value in one of the dropdowns affects the options of the ones below. So it's important that we display first the source country, then the commodity, then the change unit and finally the year. In summary, I think you should bring back the previous order.
  • Do you think you could make sure that CO2 is always displayed as CO₂? Look at this example.
  • There are two React errors in components/icons/index.js because you're using the stroke-width and stroke-linecap props instead of strokeWidth and strokeLinecap.
  • There is a further React error in components/header/menu/index.js due to the inert prop.
  • Could you update the meta tags with the new title and description?

@clementprdhomme clementprdhomme merged commit a862dcf into master Nov 17, 2023
@clementprdhomme clementprdhomme deleted the feature/restyling-chart-units branch November 17, 2023 08:01
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.

2 participants