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

Fellowship Directory Updates Mashup #3027

Merged
merged 5 commits into from
Apr 22, 2019
Merged

Fellowship Directory Updates Mashup #3027

merged 5 commits into from
Apr 22, 2019

Conversation

youriwims
Copy link
Contributor

@youriwims youriwims commented Apr 17, 2019

This pr contains the following:

  • Type update for the fellow's profile card location (Issue 1)
  • LinkedIn link typo fix (Issue 2)
  • Updating the profile cards layout at table view to match what's on mobile (Issue 3)

Test Page (all changes can be viewed here).

More detail 👇 if you'd like to keep reading 📖


Issue 1

Closes #3000
Related PRs/issues #2803 , #2810

Investigation/Discovery 👽
...attempted to track down 🛤 what happened here, but can't seem to find 🔍 exactly where and how this bug 🐛 happened. Originally, when the component was built 🔨 , it was using the older type styles for .h6-heading, but this was all fixed in the type clean up 🍃 in #2810 and reestablished as .body-small. Somehow 👀 this work reverted back to .h6-heading which resulted in it taking on the newly established .h6-heading styles reflected in the updated style guide. 🙃

Fix
Commit: Reverted styles back to .body-small based on finalized Redpen design.


Issue 2

LinkedIn link not working once you click to another year on the filter bar.
Comment Reference

Fix
Commit


Issue 3

Closes #3001
Related PRs/issues #2685

Fix
Commit: Updated layout between 768px and 992px.

@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-3027 April 17, 2019 14:55 Inactive
@youriwims
Copy link
Contributor Author

youriwims commented Apr 17, 2019

@kristinashu , I updated the styles based on the final Redpen design. But, let me know if you would like to keep the .h6-heading/uppercase typography instead for the fellow's location—it's a simple fix 😊

@youriwims youriwims force-pushed the location-uppercase branch from 1f81ff8 to bf45bfc Compare April 17, 2019 15:35
@youriwims youriwims temporarily deployed to foundation-mofostaging-pr-3027 April 17, 2019 15:35 Inactive
@kristinashu
Copy link

Looks good! Type style looks good and it's no longer switching between uppercase and lowercase!

BUT I've noticed a new bug that is here and on prod. LinkedIn link works on first page load but then doesn't work if I click on a different year. I feel like this bug keeps coming up!

@youriwims
Copy link
Contributor Author

The devs have talked about this already (will insert link to discussion once I find it 🙃 ). The directory is being loaded via two files and every time we make an update to it, we have to remember to update both files. I'll go ahead and fix that now, thanks for pointing this out.

@youriwims youriwims temporarily deployed to foundation-mofostaging-pr-3027 April 17, 2019 16:46 Inactive
@youriwims youriwims changed the title Updating location type Fellowship Directory Updates Mashup Apr 17, 2019
@kristinashu
Copy link

It's working well!

@youriwims youriwims temporarily deployed to foundation-mofostaging-pr-3027 April 17, 2019 18:27 Inactive
@youriwims
Copy link
Contributor Author

Pulse headshots have been cropped already, so the updates in Issue 3 of this pr should be uniform across all profile cards. (Reference)

@youriwims youriwims requested a review from kristinashu April 17, 2019 19:07
Copy link

@kristinashu kristinashu left a comment

Choose a reason for hiding this comment

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

Nice!

@youriwims youriwims requested a review from Pomax April 17, 2019 20:07
Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

👍

@youriwims youriwims temporarily deployed to foundation-mofostaging-pr-3027 April 22, 2019 18:19 Inactive
@youriwims youriwims force-pushed the location-uppercase branch from c3fdbbc to 514efba Compare April 22, 2019 18:25
@youriwims youriwims temporarily deployed to foundation-mofostaging-pr-3027 April 22, 2019 18:26 Inactive
@youriwims youriwims requested review from Pomax and removed request for Pomax April 22, 2019 18:31
@youriwims youriwims merged commit ce59cad into master Apr 22, 2019
@youriwims youriwims deleted the location-uppercase branch April 22, 2019 19:33
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.

Fellowships Directory layout at tablet width Fellowships directory h6-heading bug
4 participants