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

Removes odd double background color for typical code markup #7369

Merged
merged 2 commits into from
May 23, 2024

Conversation

mstahv
Copy link
Member

@mstahv mstahv commented Apr 29, 2024

Code snippets in Vaadin apps look weird without this fix. @jouni says this fixes them all 💪

Example:
screenshot_2024-04-24_at_17 19 42_720

That kind of html is generated by e.g. all markdown generators

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.
  • I have not completed some of the steps above and my pull request can be closed immediately.

Code snippets in Vaadin apps look weird without this fix.
@mstahv
Copy link
Member Author

mstahv commented Apr 29, 2024

I don't see any dot, what the heck is that complaint 🤷‍♂️

Removing spaces because it is important 🤷‍♂️
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mstahv
Copy link
Member Author

mstahv commented Apr 29, 2024

Apparently

`.`

means space.

@yuriy-fix yuriy-fix assigned web-padawan and unassigned web-padawan Apr 30, 2024
@yuriy-fix yuriy-fix requested a review from web-padawan April 30, 2024 07:56
@mstahv
Copy link
Member Author

mstahv commented May 22, 2024

Please get this fixed before the next minor, I wouldn't be surprised if somebody considers this a breaking change.

In related discussions, getting rid of the whole gray background might make sense as well (and then this change would be obsoleted). As such it sometimes causes mediocore results, even if used alone wiht only pre element:

Screenshot 2024-05-22 at 12 13 41

Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM. I think we can consider this as behavior altering fix.

@web-padawan web-padawan merged commit 27ffd4f into main May 23, 2024
9 checks passed
@web-padawan web-padawan deleted the mstahv-patch-1 branch May 23, 2024 11:54
web-padawan pushed a commit that referenced this pull request May 24, 2024
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.5.0.alpha1 and is also targeting the upcoming stable 24.5.0 version.

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.

5 participants