Make alignment great again #2450

2019-04-28T21:44:30+02:00

neb-b commented

2019-04-28 21:44:30 +02:00

(Migrated from github.com)

For https://github.com/lbryio/lbry-desktop/issues/2412

~~Dependent on https://github.com/lbryio/components/pull/8 and https://github.com/lbryio/components/pull/9~~

Changes

Tried to simplify the header/button styles
- Removed excessive html markup
- Tried to get rid of as many top: {x}px type styles as I could
Brought back the round buttons
- Square buttons are hard to make look polished
- A lot of extra styles are needed to really differentiate buttons from inputs/static markup

For https://github.com/lbryio/lbry-desktop/issues/2412 ~Dependent on https://github.com/lbryio/components/pull/8 and https://github.com/lbryio/components/pull/9~ #### Changes - Tried to simplify the header/button styles - Removed excessive html markup - Tried to get rid of as many `top: {x}px` type styles as I could - Brought back the round buttons - Square buttons are hard to make look polished - A lot of extra styles are needed to really differentiate buttons from inputs/static markup

kauffj (Migrated from github.com) approved these changes

2019-05-02 22:39:45 +02:00

kauffj (Migrated from github.com) left a comment

Mergeable

src/ui/component/button/view.jsx

					
				@ -21,4 +21,3 @@

				  type: string,

				  button: ?string, // primary, secondary, alt, link

				  noPadding: ?boolean, // to remove padding and allow circular buttons

				  iconColor?: string,

Nice. We should kill properties that are style only like this whenever possible.

src/ui/scss/component/_header.scss

					
				@ -10,1 +7,4 @@

				  z-index: 2; // Main content uses z-index: 1, this ensures it always scrolls under the header

				  background-color: $lbry-white;

				  border-bottom: 1px solid $lbry-gray-1;

Not necessary to adjust in this PR, but it can be good to make your z-index values SASS variables for better clarity.

src/ui/scss/component/_icon.scss

					
				@ -0,0 +2,4 @@

				// Minor adjustments to ensure icons line up vertically

				.icon--Heart {

				  top: -1px;

How will this scale when icons are rendered at different sizes?

neb-b (Migrated from github.com) reviewed

2019-05-02 23:45:49 +02:00

src/ui/scss/component/_icon.scss

					
				@ -0,0 +2,4 @@

				// Minor adjustments to ensure icons line up vertically

				.icon--Heart {

				  top: -1px;

That's a good point. This won't scale well. I think it will work if we use em. Currently our icons are all locked in at the same size, regardless of font size. Will keep that in mind going forward.

That's a good point. This won't scale well. I think it will work if we use `em`. Currently our icons are all locked in at the same size, regardless of font size. Will keep that in mind going forward.

Sign in to join this conversation.

No reviewers

kauffj