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

feat: simplified example and make the comparison more prominent #37065

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fuchunhui
Copy link
Contributor

Description

Motivation

Additional details

Related issues and pull requests

@fuchunhui fuchunhui requested a review from a team as a code owner December 3, 2024 12:10
@fuchunhui fuchunhui requested review from chrisdavidmills and removed request for a team December 3, 2024 12:10
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs size/m [PR only] 51-500 LoC changed labels Dec 3, 2024
Copy link
Contributor

github-actions bot commented Dec 3, 2024

Preview URLs

(comment last updated: 2024-12-13 10:55:10)

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

Hi there @fuchunhui, and thank you for your contribution to MDN.

So, while I like the idea of having the examples combined into one so you can see them side-by-side for comparison purposes, I do have some problems with your change that I think need to be solved before we can accept this:

  1. I think having two separate squares of the same size, with only a portion of the squares overlapping, is helpful to see the effect of the blend is. I think you should do this in your example, like it was in the original examples.
  2. The text labels you have given each example are not readable at all. The paragraphs need a contrasting background and foreground color, and maybe a little bit of padding.
  3. The examples are too close together — it is currently hard to see the differences between the examples and it hurts my eyes to look at a big wall of stripes.
  4. Both the comparison examples on the page use background-blend-mode. Maybe you could change yours to use mix-blend-mode instead?

@fuchunhui
Copy link
Contributor Author

Thanks for your feedback. I will solve these problems later.

@fuchunhui fuchunhui marked this pull request as draft December 10, 2024 09:44
@fuchunhui
Copy link
Contributor Author

Before After
image image

I have re-uploaded the image resources and reduced their size to more conveniently display more examples within the limited screen space. The main goal was to resolve issues with overlapping areas and increase the display spacing.

But for some reason, in the localhost environment, it's still using image dimensions of 300x300 pixels.

image

@fuchunhui
Copy link
Contributor Author

fuchunhui commented Dec 13, 2024

mix-blend-mode

Added an additional example, using mix-blend-mode. and fix the style issue in the previous example.

@fuchunhui fuchunhui marked this pull request as ready for review December 13, 2024 10:55
@chrisdavidmills
Copy link
Contributor

Hi @fuchunhui, and thanks for your work on this.

I pulled your branch locally and played around with it to see if I could figure out what is going on. I noticed that the images are saved with some whitespace in them. I edited them to remove the whitespace and just contain the 100px squares, and that made the examples behave less strangely.

I also tried giving the HTML on the "normal" example a very specific class to select, in case it was pulling in styles from somewhere else:

<div class="bmexample"></div>

I also tried setting some CSS styles to try to make sure the background images were not being repeated and were being constrained to their intrinsic size, just in case:

.bmexample {
  width: 200px;
  height: 200px;
  background-image: url("br.png"), url("tr.png");
  background-position: 25px 25px, 75px 75px;
  background-repeat: no-repeat;
  background-size: 100px 100px;
  background-blend-mode: normal;
  background-color: #dde;
}

But MDN is definitely doing something weird to the styling on this page. When I run the above code in a static HTML file through a basic HTTP server, it looks like this:

Screenshot 2024-12-13 at 15 43 07

But when I run exactly the same code inside an MDN live sample on this page (on my local environment), it looks like this:

Screenshot 2024-12-13 at 15 43 17

I will flag this to the developer team to see if they have got any idea what is going on. @fiji-flo @caugner, any thoughts? I think we should pause work on this demo until we've investigated this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants