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

Scorecards 2023 initial views #533

Merged
merged 50 commits into from
Sep 4, 2023

Conversation

struan
Copy link
Member

@struan struan commented Jun 27, 2023

  • split out the old 2022 scorecards to /plan-scorecards-2022/
  • remove question view from new scorecards
  • add section view
  • switch new views to bootstrap 5

This requires a docker image rebuild to get bootstrap 5.

Fixes #532.

@zarino zarino force-pushed the master branch 2 times, most recently from d0a4b2b to ec6d34c Compare June 30, 2023 13:23
@lucascumsille
Copy link
Contributor

Hi @zarino
Still a work in progress, but basically I'm trying to get rid as much unnecessary code as possible. Not sure if you'd rather me to change this as a draft, and I keep tidying up after I'm back and then ask you for a proper feedback or you can take a look of what I've done so far.

@lucascumsille
Copy link
Contributor

Current state:

Screen.Recording.2023-07-18.at.09.02.38.mov

@zarino
Copy link
Member

zarino commented Jul 18, 2023

Not sure if you'd rather me to change this as a draft, and I keep tidying up after I'm back and then ask you for a proper feedback or you can take a look of what I've done so far.

Thanks @lucascumsille! I’ll do my best to take a look while you’re away!

@zarino zarino self-assigned this Jul 18, 2023
Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

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

A huge amount of work here, and overall really well done. So refreshing to see us make much better use of built-in Bootstrap styles, rather than rebuilding the wheel everywhere like last time.

Some design feedback for @lucascumsille (we can discuss this next week!):

Header

  • I’d replace the "2023 Action Scorecards" <select> element with a bootstrap dropdown. Probably doesn’t even need to look like a "button" – it’s really just a nav link with a dropdown menu.
    • (Will need to think about how this dropdown works without JavaScript. I have some ideas.)
    • I’d also just show the year in the dropdown trigger itself, and leave the full titles for the dropdown items (same way the button and menu item texts don’t match on the TWFY leglisature picker, eg: https://www.theyworkforyou.com/senedd/)
    • Bonus points if we can have the dropdown items link to the "equivalent" page from the chosen year – eg if you’re looking at the 2023 County Council scores, the 2021 dropdown item would link to the 2021 County Council scores page rather than 2021 homepage.
  • Nav doesn’t respond well at medium page widths. Can we break the nav links onto a line above or below the search input, when there isn’t room to have them side by side?
  • On narrow screens, the Bootstrap default .navbar { justify-content: space-between; } is leaving the "2023 Action Scorecards" dropdown element stuck in the middle of nowhere.
  • Search input doesn’t look like a search input.
  • Do we have thoughts on colours here? I assume bright blue is a placeholder?

Footer

  • Last year’s Scorecards footer was a mess, and this year we look set to continue that tradition! I wonder whether we should break out the newsletter widget into its own section (with a lighter background) before the footer, so the footer itself can be much shorter, with just the logos, nav links, and external links?

And some further comments about bits of the homepage, which I guess technically fall under #513, but you seem to have done some styling of the homepage content in this branch already, so feedback on that:

"Advanced filters" modal

  • I think the "Apply filters" button should be inside .modal-footer
  • And to avoid confusion, should "Close" be "Cancel" instead? (I assume clicking this button will close the modal and throw away any changes you’ve made?)
  • I wonder whether we can show when changes are active in each accordion section? (So you know where you might have previously set active filters.)

Homepage table

  • "Average" row overlaps header row on scroll (Firefox 115).
  • .scorecard-table th { vertical-align: initial } is messing up the vertical alignment of progress bars in the "Total score" column. Perhaps use .scorecard-table thead th instead?

@lucascumsille
Copy link
Contributor

lucascumsille commented Aug 16, 2023

Feedback checklist

Header

  • I’d replace the "2023 Action Scorecards" <select> element with a bootstrap dropdown. Probably doesn’t even need to look like a "button" – it’s really just a nav link with a dropdown menu.

    • (Will need to think about how this dropdown works without JavaScript. I have some ideas.)
    • I’d also just show the year in the dropdown trigger itself, and leave the full titles for the dropdown items (same way the button and menu item texts don’t match on the TWFY leglisature picker, eg: https://www.theyworkforyou.com/senedd/)
    • Bonus points if we can have the dropdown items link to the "equivalent" page from the chosen year – eg if you’re looking at the 2023 County Council scores, the 2021 dropdown item would link to the 2021 County Council scores page rather than 2021 homepage.
  • Nav doesn’t respond well at medium page widths. Can we break the nav links onto a line above or below the search input, when there isn’t room to have them side by side?
    NOTE: On mobile should we add the searcher to the collapsible menu? or still leave it outside?

  • On narrow screens, the Bootstrap default .navbar { justify-content: space-between; } is leaving the "2023 Action Scorecards" dropdown element stuck in the middle of nowhere.

  • Search input doesn’t look like a search input.

  • Do we have thoughts on colours here? I assume bright blue is a placeholder?

Footer

  • Last year’s Scorecards footer was a mess, and this year we look set to continue that tradition! I wonder whether we should break out the newsletter widget into its own section (with a lighter background) before the footer, so the footer itself can be much shorter, with just the logos, nav links, and external links?

And some further comments about bits of the homepage, which I guess technically fall under #513, but you seem to have done some styling of the homepage content in this branch already, so feedback on that:

"Advanced filters" modal

  • I think the "Apply filters" button should be inside .modal-footer
  • And to avoid confusion, should "Close" be "Cancel" instead? (I assume clicking this button will close the modal and throw away any changes you’ve made?)
  • I wonder whether we can show when changes are active in each accordion section? (So you know where you might have previously set active filters.)

Homepage table

  • "Average" row overlaps header row on scroll (Firefox 115).
  • .scorecard-table th { vertical-align: initial } is messing up the vertical alignment of progress bars in the "Total score" column. Perhaps use .scorecard-table thead th instead? NOTE: Tried that, but the wouldn't solve the issue with the other columns. If we use vertical-align: top works fine on Safari, Firefox and Chrome.

@lucascumsille
Copy link
Contributor

@zarino I have tackled the feedback above, plus extra stuff like spacing and getting rid of unnecessary code.
Some bullets points from #513 still need to be addressed later on. So I would think this PR Fixes: #511

Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

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

@struan I’ve prepared this branch for merging, and was about to merge to master, but then thought – do we actually want to merge all of the WIP Scorecards 2023 stuff into master while we’re working on it, or should we be working in a development branch? Not sure whether merging to master would mean the WIP scorecards site gets deployed whenever we deploy CAPE, for example 😬

Figured it’s best to leave until you’re back!

PS I’m aware the commit history for this branch is a bit of a mess, but squashing and reordering 50 commits doesn’t feel like a great use of anyone’s time. Let me know if you disagree!

@zarino zarino assigned struan and unassigned zarino Sep 1, 2023
lucascumsille and others added 27 commits September 4, 2023 10:57
- Remove "Scorecards" from button label, leaving just year
- Style button as a nav link
- Add drop shadow to dropdown
- Add absolute position to dropdown even on small screens
- Add TODO comments about missing URLs
- New vertical layout for nav search input and nav links
- Smaller nav search input
- Proper label for the search input, rather than CSS pseudo-element
- Add hidden submit button to search form
- Less garish styling for search input label
- Switch out of mobile nav at lg, rather than xl, breakpoint
- Slightly more horizontal padding around header on wider screens
- Remove "New" from "New Methodology" nav link text
- Add _utils.scss to main.scss import flow
- Move .text-bg-* declarations out of main.scss into _utils.scss
- Move $spacers declaration out of main.scss into variables.scss
- Comment out unused Bootstrap component imports
@zarino zarino force-pushed the scorecards-2023-initial-views branch from b9564ce to 37c9f16 Compare September 4, 2023 09:58
@zarino zarino changed the base branch from master to scorecards-2023 September 4, 2023 10:00
@lucascumsille lucascumsille merged commit 37c9f16 into scorecards-2023 Sep 4, 2023
5 checks passed
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.

Setup basic views etc for 2023
3 participants