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

Combine vendor and contract columns #2112

Merged
merged 6 commits into from Sep 4, 2018

Conversation

Projects
None yet
3 participants
@hbillings
Member

hbillings commented Aug 31, 2018

Closes #2034.

screen shot 2018-08-31 at 3 42 52 pm

To do:

  • Update tests

@hbillings hbillings requested review from tadhg-ohiggins and ericronne Aug 31, 2018

@@ -32,7 +32,7 @@ export const createDataCellConnector = key => (Component) => {
};
export function GenericDataCell({ className, value }) {
export function GenericDataCell({ className, value, result = {} }) {

This comment has been minimized.

@hbillings

hbillings Aug 31, 2018

Member

This is kind of funky: in order to access the contract number for a given result, I had to pass the result explicitly into GenericDataCell. But every table cell inherits this class, and none of the other cells besides VendorName need awareness of the entire result object. So I passed a blank object by default, to avoid lots of unnecessary argument-passing. 🤷‍♀️

I'm open to other ways to do this!

@ericronne

Looks great except for a jiggle on hover that's caused by the line height of the word "years." Reducing the line height to 12px for .years should do the trick, lmk if you want me to handle.

ericronne and others added some commits Sep 4, 2018

Fix for the hover jiggle
Line height was causing rows with only two lines of info to grow taller on hover.

@hbillings hbillings changed the title from [WIP] Combine vendor and contract columns to Combine vendor and contract columns Sep 4, 2018

@hbillings

This comment has been minimized.

Member

hbillings commented Sep 4, 2018

@ericronne Since your requested changes were addressed in #2113, I'm going to go ahead and merge this. 🖖

Addressed in #2113

@hbillings hbillings merged commit 37a374d into develop Sep 4, 2018

3 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
security/snyk - package.json (CALC) No manifest changes detected
security/snyk - requirements.txt (CALC) No manifest changes detected

@hbillings hbillings deleted the 2034-combine-2-columns branch Sep 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment