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

Fix border bottom CSS var style #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benri
Copy link
Contributor

@benri benri commented Jun 1, 2021

Fixes border property overriding the style of border-bottom when the CSS variable is undefined

Fixes issue introduced by this commit ded3611#diff-eccc2b8fef024888c50f0586687e68415115c3cc38bd9fda26ba1d4cbd0ce0dcR20

border: var(--api-body-document-title-border);
border-bottom: var(--api-body-document-title-border-bottom,
1px var(--api-body-document-title-border-color, var(--api-parameters-document-title-border-color, #e5e5e5)) solid
);
Copy link
Contributor

@deiteris deiteris Jun 2, 2021

Choose a reason for hiding this comment

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

@benri, what's the initial idea for border styling actually? To make it possible to modify the entire border-bottom property, or being able to also modify the color of all border sides? If it's about sides, it can be done in the following way:

border: var(--api-body-document-title-border, 1px solid);
border-color: var(--api-body-document-title-border-color-full, transparent var(--api-parameters-document-title-border-color, #e5e5e5) transparent transparent);

This way the default behavior is preserved while customization of border becomes more flexible:

  1. The border is set to 1px solid if variable is not defined and we can also modify the appearance of the border.
  2. The border color is set to transparent for all border sides except bottom by default. It can be modified in any way we want. F.e., I can set --api-body-document-title-border-color-full to #FFF #FFF, and this will paint top and bottom borders white.

Notice that I named variable as --api-body-document-title-border-color-full, because there's a variable name clash. I am not sure how it should be handled in this case but maybe other contributors know better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For my use case, I am only styling the border-bottom (actually, hiding it by setting border-bottom: 0) so I may not need to color the other border sides

Copy link
Member

Choose a reason for hiding this comment

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

@deiteris @benri I am working on a consolidation of all documentation components under a single repository. You can see the new code base here: https://github.com/advanced-rest-client/api-documentation/tree/release/7.0.0
It is not yet ready as I am still merging test cases from the old components. This will be a breaking change anyway so it will be easier to make major styling changes out there. I'll be happy to hear your feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jarrodek since you're moving some stuff from api-type-document there as well, please also take a look at the issues and PRs here:
https://github.com/advanced-rest-client/api-type-document/issues

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

Successfully merging this pull request may close these issues.

3 participants