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

Improve styles and readability #195

Closed
wants to merge 298 commits into from
Closed

Conversation

arieltonglet
Copy link
Contributor

@arieltonglet arieltonglet commented Mar 25, 2020

Description

Improvements to the general user experience described at #83 :

  • Alter font weights in titles and subtitles to improve hierarchy

  • Give form inputs clear visual distinction by using white background color

  • Add instructive subtitles to modules (“Use presets”, “Input data below” etc)

  • Push help buttons to the right side of labels and change its color/opacity to a more subtle one (from light blue to a light grey)

  • Add h1 to main title “COVID-19 Scenarios” and proper semantic tags for subtitles (h2, h3 and so on), as well as “main”, “article”, “section” etc where applicable

  • The text on the "severity assumptions" module is overflowing on mobile. We should wrap it in its div

  • Add a spinner to “Run simulation” button, to indicate to the user that the simulation is being created/updated

Related issues

Implements the design improvements proposed at #83
Related: #13 #

Impacted Areas in the application

Main.tsx
Changes to bootstrap's props and classes

ResultsCard.tsx
ScenariosCard.tsx
Changes to bootstrap's props and classes, altered subtitles tags

ScenarioCardContainment.tsx
ScenarioCardEpidemiological.tsx
ScenarioCardPopulation.tsx
ScenarioCardPopulation.tsx
Altered subtitles tags

FormHelpButton.tsx
HelpLabel.tsx
Changes to classes and elements orders

global.scss
FormHelpButton.scss

Testing

@vercel
Copy link

vercel bot commented Mar 25, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/covid19-scenarios/covid19-scenarios/kibja0a2d
✅ Preview: https://covid19-scenarios-git-fork-arieltonglet-master.covid19-scenarios.now.sh

@gj262
Copy link
Collaborator

gj262 commented Mar 25, 2020

Looks great IMO! One issue in the form though (left hand side, bottom) the card subtitle it not readable:
Screen Shot 2020-03-24 at 9 51 36 PM

@arieltonglet
Copy link
Contributor Author

Thanks, @gj262 ! (let me cc @rodolfoalmeida01 here)
I have yet to tackle the inner elements of the cards (basically everything but the headings, so far). You can check what I'm aiming for at #83 :)

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Mar 25, 2020

@arieltonglet @rodolfoalmeida01 Wow! I don't recognize this beauty! Fantastic work!

@ivan-aksamentov ivan-aksamentov added pr: needs rebase This pull request has to be updated in order to be merged pr: work in progress This is work in progress, do not merge until finished and removed pr: needs rebase This pull request has to be updated in order to be merged labels Mar 25, 2020
@arieltonglet
Copy link
Contributor Author

arieltonglet commented Mar 31, 2020

hi guys, i think i finished the last adjustments for the general styles.
As there are some new elements and others that have been removed, I had to make some changes in relation to what @rodolfoalmeida01 had proposed. Anyway, I think the general idea remained.

A major change, which was not "formally" planned in the layout, was in the help tooltips.
The way they are coded now, the "label" prop, which makes the tooltip title, can be a string, a h2/h3 or even a fragment with multiple children tags inside (Severity card). This makes the help title receive different tags, and have different styles, depending on the element it is referencing.

So I made some adjustments to FormHelpButton.tsx and now the "label" prop is being filtered to get the text content only, which goes to the same <h4> for all the help tooltips. Since typescript is not exactly my comfort zone, I ask you to take a look if that code is as expected.

(Maybe a better approach would be to edit the props that are being sent to FormHelpButton, but I didn't think that it was the scope for this PR, as I was trying to stick my editions as much as I could to stuff like classNames and styles only)

@ivan-aksamentov ivan-aksamentov marked this pull request as ready for review March 31, 2020 08:28
@ivan-aksamentov
Copy link
Member

@arieltonglet This is fantastic!
The only thing is that the diff is like 116 files... most of these changes are already on master.
Not sure what will happen if I merge.

@gj262 @rcbevans Is there a way for us to help @arieltonglet and unscrew the diff on our side somehow?

@rneher
Copy link
Member

rneher commented Mar 31, 2020

I really like the new look! Excited to see this. One thought I had is whether we want title bars and lines to be pitch black or some sort of dark grey. The navigation bar is grey. But that is minor and I am happy to go with other people's judgment here.
No sure about the diffs.

@arieltonglet
Copy link
Contributor Author

@arieltonglet This is fantastic!
The only thing is that the diff is like 116 files... most of these changes are already on master.
Not sure what will happen if I merge.

Ok, I thought I was rebasing the right way but clearly it wasn't the case. Sorry I'm a little lost here. If you think its better, I could review my commits and start a clean slate RP (following better practices this time 🙃 )

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Mar 31, 2020

@arieltonglet Yes, this would be awesome. Try to just check out master, copy your final stuff over, commit and make a PR. We just need a reasonably readable diff. Don't bother with any fancy stuff, like commit history etc.

I postpone merging anything to master meanwhile so that it stays in sync.

Thanks so much!

@ivan-aksamentov
Copy link
Member

@arieltonglet I rebased and merged in #336. Thanks a lot for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPORTANT Take this immediately! pr: needs rebase This pull request has to be updated in order to be merged pr: work in progress This is work in progress, do not merge until finished
Projects
None yet
Development

Successfully merging this pull request may close these issues.