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

3071 Org chart styles #3130

Merged
merged 5 commits into from
Sep 19, 2019
Merged

3071 Org chart styles #3130

merged 5 commits into from
Sep 19, 2019

Conversation

rfultz
Copy link
Contributor

@rfultz rfultz commented Aug 27, 2019

Summary

Outlines the structure for a responsive org chart and styles it according to comps

Impacted areas of the application

Right now, the only change is a new scss file that will apply to an upcoming org chart.
That file is included in fec/fec/static/scss/base.scss for now (see the TODOs at the bottom).

Screenshots

image
image

Related PRs

None

How to test

  • Pull the branch
  • npm run build just in case (if you get an error, you may need to run npm rebuild node-sass then build again)
  • Copy the sample html from the bottom of fec/fec/static/scss/components/_fec-org-chart.scss
  • I'm still new to Wagtail; I think this is what I did:
    • In local Wagtail, find a page to edit (I chose Home > About > Leadership and structure)
    • Add a new html section
    • Paste the copied html into the new section
    • Save draft
    • Preview
  • Scale your browser window from its smallest width across all the sizes to 980px+

TODO:

  • @JonellaCulmer, is the hover too subtle—should we make it stronger?
  • Build wagtail template for organization chart #3072 will work out how the org chart lives on the site; we may need to move the css then.
  • We need a wider page template
  • Does it still make sense to include _fec-org-chart.scss in base.scss or would it be better somewhere else?

@JonellaCulmer
Copy link
Contributor

@rfultz In your instructions, we also need to run npm run build-sass, or at least I had to.

Copy link
Contributor

@JonellaCulmer JonellaCulmer left a comment

Choose a reason for hiding this comment

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

Looks good, including the rollover state. Thanks!

Copy link
Contributor

@dorothyyeager dorothyyeager left a comment

Choose a reason for hiding this comment

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

@JonellaCulmer asked me to look at it from content team perspective. It seems OK but be aware that depending on what we find in our focus groups #3103 , we may end up coming back to the organization chart design. Also, picky picky, will need to take the "Acting" out of Neven's title when we get to the real chart. Finally, are the footnotes incorporated into this?

@rfultz
Copy link
Contributor Author

rfultz commented Sep 3, 2019

@rfultz In your instructions, we also need to run npm run build-sass, or at least I had to.

Oops. Sorry. I've updated it.

@JonellaCulmer
Copy link
Contributor

@dorothyyeager

are the footnotes incorporated into this?

I assumed we'd retain the current process we use for footnotes. There would just need to be another text block after the org chart, which is something @johnnyporkchops will be able to contend with on the org chart template. When we create a template for footnotes, I'd like them to be the same across the whole site.

@JonellaCulmer JonellaCulmer added this to the Sprint 10.1 milestone Sep 5, 2019
@patphongs patphongs removed their request for review September 16, 2019 15:35
@johnnyporkchops
Copy link
Contributor

@rfultz . To answer your question, I think it makes sense to import_fec-org-chart.scss in base.scss
Also, for the full-width page template --That template will be part of the reporting-dates tables PR which is getting closer and closer to being complete ... if it is not, before they are ready to make the org chart html block, I can do a small separate PR to get that available in Wagtail ahead of time.

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (develop@9aee54b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #3130   +/-   ##
==========================================
  Coverage           ?   74.76%           
==========================================
  Files              ?      119           
  Lines              ?     7158           
  Branches           ?      633           
==========================================
  Hits               ?     5352           
  Misses             ?     1806           
  Partials           ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9aee54b...f6743fc. Read the comment docs.

Copy link
Contributor

@johnnyporkchops johnnyporkchops left a comment

Choose a reason for hiding this comment

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

Looks /works great. Nice clean html structure and concise SCSS!

@patphongs
Copy link
Member

All reviewers have approved, merging...

@patphongs patphongs merged commit 323b1a5 into develop Sep 19, 2019
@lbeaufort lbeaufort deleted the feature/3071-org-chart branch November 29, 2019 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTML/responsive-side work for organizational chart
6 participants