LBRYCommunity/components

Fork 0

Make links easier to override #7

Merged

neb-b merged 1 commit from button--link-1 into master

2019-03-11 16:20:16 +01:00

neb-b commented

2019-03-07 19:29:25 +01:00

(Migrated from github.com)

I will probably end up making a few more of these as I run into them. The current setup makes it really difficult to override styling due to the high specificity of the default state.

This moves the color to the top level, then adds individual blocks for :hover, and :disabled

I will probably end up making a few more of these as I run into them. The current setup makes it really difficult to override styling due to the high specificity of the default state. This moves the `color` to the top level, then adds individual blocks for `:hover`, and `:disabled`

NetOpWibby (Migrated from github.com) reviewed 2019-03-07 19:33:11 +01:00

NetOpWibby (Migrated from github.com) left a comment

In what situation would you need to override these? And, with what?

neb-b commented

2019-03-07 19:43:42 +01:00

(Migrated from github.com)

@NetOperatorWibby The splash screen in the app. We turn all links white because of the green background. This makes it so we just need to add a separate class load-screen__button and the color will work.

Instead of having to do something like:

.button--link.load-screen__button:not(:disabled):not(:hover) { color: $lbry-white }

Another place we do (and is a pain) is buttons inside of forms. But I figured I would make a PR with this since it's smaller to figure out the best way forward.

@NetOperatorWibby The splash screen in the app. We turn all links white because of the green background. This makes it so we just need to add a separate class `load-screen__button` and the color will work. Instead of having to do something like: ``` .button--link.load-screen__button:not(:disabled):not(:hover) { color: $lbry-white } ``` Another place we do (and is a pain) is buttons inside of forms. But I figured I would make a PR with this since it's smaller to figure out the best way forward.

NetOpWibby commented

2019-03-07 19:56:12 +01:00

(Migrated from github.com)

I don't think that's a reason to change the component logic. Loading screens are special cases, it makes sense to have to do specific things that only apply to them, within them.

neb-b commented

2019-03-07 20:20:26 +01:00

(Migrated from github.com)

I'm not changing the logic of this though. I'm just removing un-needed specificity.

NetOpWibby commented

2019-03-07 20:29:23 +01:00

(Migrated from github.com)

The specificity applies to buttons that are not disabled. Disabled buttons shouldn't have styling applied to them. Component styles should not be overridden.

Why not just add an extra class for button links that should be white?

The specificity applies to buttons that are not disabled. Disabled buttons shouldn't have styling applied to them. Component styles should not be overridden. Why not just add an extra class for button links that should be white?

neb-b commented

2019-03-07 22:19:16 +01:00

(Migrated from github.com)

I removed the :disabled style. I realize that is not needed with pointer-events: none on disabled buttons.

I don't see why these can't be overridden though. I want the same style rules for it, but only want to change the color. If I need to use an entirely different class instead of combining them I will end up with duplicated code.

This change makes it so I can do this:

<button class="button--link load-screen__button">...

.load-screen__button {
  color: $lbry-white;

  &:hover {
    color: $lbry-blue-1;
  }
}

instead of

.load-screen__button:not(:disabled) {
  &:not(:hover) {
    color: $lbry-white;
  }

  &:hover {
    color: $lbry-blue-1;
  }
}

I removed the `:disabled` style. I realize that is not needed with `pointer-events: none` on disabled buttons. I don't see why these can't be overridden though. I want the same style rules for it, but only want to change the color. If I need to use an entirely different class instead of combining them I will end up with duplicated code. This change makes it so I can do this: ``` <button class="button--link load-screen__button">... .load-screen__button { color: $lbry-white; &:hover { color: $lbry-blue-1; } } ``` instead of ``` .load-screen__button:not(:disabled) { &:not(:hover) { color: $lbry-white; } &:hover { color: $lbry-blue-1; } } ```

NetOpWibby commented

2019-03-08 21:06:04 +01:00

(Migrated from github.com)

I suppose I can just prevent the rules for [disabled] and .button--disabled from being overridden.

The test:sass script needs to be run on this PR, then it'll be good for merge and I can deploy to npm.

I suppose I can just prevent the rules for `[disabled]` and `.button--disabled` from being overridden. The `test:sass` script needs to be run on this PR, then it'll be good for merge and I can deploy to npm.

neb-b commented

2019-03-09 00:31:13 +01:00

(Migrated from github.com)

@NetOperatorWibby Ran test:sass. Should be good I think.

@NetOperatorWibby Ran `test:sass`. Should be good I think.

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/components#7

No description provided.