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

Cleanup group Selector #52

Merged
merged 3 commits into from
Sep 27, 2018

Conversation

JordanSjodinFaithlife
Copy link
Contributor

This PR contains a number of fixes:

One is it removed some duplicated code and switched from using float and hardcoded padding to center things to using flex box.

Also, if removed some styles that were being set directly on components instead of using styled components.

Lastly, making a complicated component that works on everyone's web page is hard. 'p' elements were used in this component and were getting useragent styles with margins such that it looked nice in the catalog, but could look bad elsewhere. Switching to divs works around this and seems better since the 'p' elements weren't really being used for paragraphs of text.

@dustinsoftware

Switched to using flexbox.
Removed hardcoded, top padding meant to center text
Merged two sets of StyledComponents that were very similar

Merge with clean up
Clean up avatar
Paragraph elements seem to get a lot of useragent style sheets applied and they are unnecessary.

sdf
font-size: 13px;
color: #585250;
white-space: nowrap;
width: ${props => props.size}px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the width and height being overridden anywhere? Or is this old code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dustinsoftware
Copy link
Contributor

There are some issues with scrollbar fading in Firefox:
image

@dustinsoftware
Copy link
Contributor

dustinsoftware commented Sep 25, 2018

In Group Selector without Groups:

When mousing over Create, the button does not change state and appears to be unclickable.
image

@JordanSjodinFaithlife
Copy link
Contributor Author

JordanSjodinFaithlife commented Sep 26, 2018

The two bugs you mentioned are present in the current version. Could I address those in a future PR? (i.e. not have them block this one)

@dustinsoftware ?

@dustinsoftware dustinsoftware merged commit 8099325 into Faithlife:master Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants