Skip to content

Refactor markup code block styles #30294

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

Closed
wants to merge 1 commit into from

Conversation

wxiaoguang
Copy link
Contributor

Close #30292

image

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 5, 2024
@wxiaoguang wxiaoguang added type/bug backport/v1.22 This PR should be backported to Gitea 1.22 labels Apr 5, 2024
@wxiaoguang wxiaoguang force-pushed the fix-markup-code-block branch from f7f969c to ac1b49f Compare April 5, 2024 15:02
@silverwind
Copy link
Member

silverwind commented Apr 5, 2024

Root of the problem is that :not(.code-inner) that was added in #30234 adds specificity which makes the selector win over .markup pre > code, honstestly I would rather like to revert this :not() addition than to layer more hacks on top of it.

We should tread careful around this markup CSS, we should stay as close as possible to GH CSS. What were you targeting with :not(.code-inner)?

BTW, I already did a different fix for this in #30282, but am not happy with it either.

.markup .highlight pre > code,
.markup pre > code {
  background: none !important;
}

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 5, 2024
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Apr 5, 2024

@silverwind I believe is good enough and it is a right fix. It clearly defines the styles for different code usages, and remove all dirty/unclear CSS style overridings, and no need to use !important.


Update:

honstestly I would rather like to revert this :not() addition than to layer more hacks on top of it

It would just introduce more overridings, and make the styles more difficult to maintain.

We should tread careful around this markup CSS, we should stay as close as possible to GH CSS. What were you targeting with :not(.code-inner)?

code-inner is for the code-preview code highlight, to keep it the same as other code renders. And it is still a good name to distinguish it from other inline/block code usages.

font-size: 85%;
white-space: break-spaces;
background-color: var(--color-markup-code-block);
border-radius: var(--border-radius);
}
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this selector list. what about blockquote > code etc, you can't possibly list every single element here.

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Would like to see .markup code:not(.code-inner) being reverted to .markup code. This is too hacky.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 5, 2024
@wxiaoguang
Copy link
Contributor Author

I don't see anything hacky. The existing code is more hacky and will cause more problems.

@wxiaoguang wxiaoguang closed this Apr 5, 2024
@silverwind
Copy link
Member

silverwind commented Apr 5, 2024

If we really have to we, can use .markup code:where(:not(.code-inner)) to avoid adding specificity in a :not selector but I would prefer to stay as close as possible to GitHub CSS and that is definitely with .markup code.

I will continue the fix in #30282 where I will revert the :not.

@wxiaoguang
Copy link
Contributor Author

I have no interesting for playing style overriding games, as always.

@wxiaoguang
Copy link
Contributor Author

-> Fix code block style for code preview #30298

I think it is better than overriding unnecessary styles.

@wxiaoguang wxiaoguang removed the backport/v1.22 This PR should be backported to Gitea 1.22 label Jun 20, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code markup style regression
4 participants