Refactor of Scoped CSS

[EliasMasche]

Initial PR of the process for Scoped CSS

[linkdotnet]

This might be "dangerous" as it now targets every "img" whereas it was scoped to the component before. so probably you want something like actions-img or so?

[EliasMasche]

Understood, gonna check that issue

[linkdotnet]

Currently the bUnit tests are red, as you "renamed" the CSS selectors

[EliasMasche]

Currently the bUnit tests are red, as you "renamed" the CSS selectors

So I rename it back to the original or update the bUnit tests?

[linkdotnet]

Currently the bUnit tests are red, as you "renamed" the CSS selectors

So I rename it back to the original or update the bUnit tests?

I would do the latter

[EliasMasche]

Removed the empty razor css files and added one that I missed.

An issue that remains is at home, the introduction hasn't show up yet of IntroductionCard.razor.css into home.css
and the CSS of PreviewImage.razor.css into components.css not being functional either

src/LinkDotNet.Blog.Web/wwwroot/css/home.css

[linkdotnet]

Removed the empty razor css files and added one that I missed.

An issue that remains is at home, the introduction hasn't show up yet of IntroductionCard.razor.css into home.css and the CSS of PreviewImage.razor.css into components.css not being functional either

src/LinkDotNet.Blog.Web/wwwroot/css/home.css

Hmm that might be because of --profile-background. Even though it should work. If you open your devtools and check the value of --profile-background is there something useful?

[linkdotnet]

There are some smaller tests that refer to the "old" structure:

 var items = cut.FindAll(".profile-keypoints li");

[EliasMasche]

Removed the empty razor css files and added one that I missed.
An issue that remains is at home, the introduction hasn't show up yet of IntroductionCard.razor.css into home.css and the CSS of PreviewImage.razor.css into components.css not being functional either
src/LinkDotNet.Blog.Web/wwwroot/css/home.css

Hmm that might be because of --profile-background. Even though it should work. If you open your devtools and check the value of --profile-background is there something useful?

Based on dev tools the profiles images are loaded correctly but don't show up on the page

How it works: -profile-picture, -profile-image, .introduction-container, .introduction-background. This is my first time seeing this, and I am super new to dealing with CSS, I feel I am missing something.

Another bug is with PreviewImageRazor for blogposts that takes over the entire space and locks

The css is img that I renamed to comp-img the code is PreviewImage.razor in the HTML code uses the img tag how to rename properly or refactor for taking into account the proper CSS code

@if (string.IsNullOrEmpty(PreviewImageUrlFallback))
{
    <img src="@PreviewImageUrl" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" />
}
else
{
    <picture>
        <source srcset="@PreviewImageUrl" type="@GetMimeType()"/>
        <img src="@PreviewImageUrlFallback" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" />
    </picture>
}

[EliasMasche]

There are some smaller tests that refer to the "old" structure:

 var items = cut.FindAll(".profile-keypoints li");

Do you need me to update them and search for other CSS renames too?

[EliasMasche]

Another bug is with PreviewImageRazor for blogposts that takes over the entire space and locks

The css is img that I renamed to comp-img the code is PreviewImage.razor in the HTML code uses the img tag how to rename properly or refactor for taking into account the proper CSS code

@if (string.IsNullOrEmpty(PreviewImageUrlFallback))
{
    <img src="@PreviewImageUrl" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" />
}
else
{
    <picture>
        <source srcset="@PreviewImageUrl" type="@GetMimeType()"/>
        <img src="@PreviewImageUrlFallback" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" />
    </picture>
}

About this portion of the issue with PreviewImage.razor I did progress adding

.img-container {
	position: relative;
	width: 100%;
	height: 100%;
	overflow: hidden;
}

.comp-img {
	position: absolute;
	top: 0;
	left: 0;
	object-fit: cover;
	height: 100%;
	width: 100%;
}

For PreviewImageUrl works as before using <div class="img-container"> and class="comp-img"

    <div class="img-container">
        <img class="comp-img" src="@PreviewImageUrl" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" /> 
    </div>

For PreviewImageUrlFallback is not working as in PreviewImageUrl.

    <div class="img-container">
        <picture>
            <source srcset="@PreviewImageUrl" type="@GetMimeType()" />
            <img class="comp-img" src="@PreviewImageUrlFallback" alt="Preview image blogpost" loading="@LazyLoadTag" decoding="@DecodingTag" />
        </picture>
    </div>

but for some reason now stopped working haha the PreviewImageUrl too

[linkdotnet]

There are some smaller tests that refer to the "old" structure:

 var items = cut.FindAll(".profile-keypoints li");

Do you need me to update them and search for other CSS renames too?

We have to make the tests work in one way or another. In this regard, the selector has to be aligned to the new "prefix" added.

[linkdotnet]

I have to checkout the branch later locally. I tried inside GitHub Codespaces and I got some 404 for the images. Not sure if this is because of GitHub Codespace or it showcases the underlying issue on why the images aren't showing up.

[linkdotnet]

@EliasMasche I took the opportunity to push to your PR. Hope you don't mind.

[EliasMasche]

@EliasMasche I took the opportunity to push to your PR. Hope you don't mind.

Yeah, no problem. the only issue that remains is the home/introduction CSS, right?

[linkdotnet]

@EliasMasche I took the opportunity to push to your PR. Hope you don't mind.

Yeah, no problem. the only issue that remains is the home/introduction CSS, right?

Technically, yes. So of course everything should work as before, but overall I would like to go a step further, otherwise we just moved the CSS from one file to another.

Ideally:

1. We can refactor many of the custom CSS classes to bootstrap 5 utility classes
2. Consolidate the rest. There is much overlap in the content of those classes which can be moved together.
3. Have then only one basic.css (well and the icon.css) to reduce the initial browser load.

[linkdotnet]

Just a small heads-up, for me the profile picture loads just fine, but the ShortBlogPost is a bit "different":

[linkdotnet]

The navbar items seem a bit closer than they were before and lost their "bold" font.

[linkdotnet]

Should also be fixed. Now the interesting part starts: Consolidating :D
That is where I see the real value in the PR and ticket

[EliasMasche]

Roger that.

I checked your commits. I pulled into my local branch, and the CSS works now, but I am getting a 404 for the images.
Reason: strict-origin-when-cross-origin

@EliasMasche I took the opportunity to push to your PR. Hope you don't mind.

Yeah, no problem. the only issue that remains is the home/introduction CSS, right?

Technically, yes. So of course everything should work as before, but overall I would like to go a step further, otherwise we just moved the CSS from one file to another.

Ideally:

1. We can refactor many of the custom CSS classes to bootstrap 5 utility classes

2. Consolidate the rest. There is much overlap in the content of those classes which can be moved together.

3. Have then only one `basic.css` (well and the `icon.css`) to reduce the initial browser load.

The end goal is to put all CSS into basics.css, refactor and reduce the custom CSS classes with bootstrap 5 utility classes.

That is where I see the real value in the PR and ticket

Good thing this time I put the PR as draft as it is more of a proposal and WIP.

[linkdotnet]

Interesting. By any chance, do you have a chrome based browser to cross-check if it happens there as well?
If so, if you go to "my website": steven-giesel.com - do you see the image? The difference is that the image on my website is hosted on a Blob storage and not as part of the application.

Good thing this time I put the PR as draft as it is more of a proposal and WIP.

No worries - that is part of the process. Issues/PR's are async in nature and therefore are more likely to have misunderstandings and whatnot.

[EliasMasche]

Yeah, same issue coming from Edge Browser,

What it dislikes is that it comes from the same application but a different domain or locally triggering CORS if I use it from another website/domain, or blob storage works fine.

[EliasMasche]

Hello there, sorry ( but may not be sorry) for the disappearance. I got relocated to a new team, a new project with an impossible deadline, so I wasn't able to work.

So now I pushed a new commit, redoing the rename of a class of CSS as it wasn't necessary, I believed they had the same name, I put them all inside basics.css

A question about the bootstrap, this sentence: We can refactor many of the custom CSS classes to Bootstrap 5 utility classes

Where can I find more information, or is it referring to the internal CSS classes of Bootstrap 5

[linkdotnet]

Hey hey - don't worry. OSS has to make space for other priorities so I totally get that and there is no need to apologize.

Where can I find more information, or is it referring to the internal CSS classes of Bootstrap 5

Sometimes I added stuff like padding: 4px or so. Which "should" be bootstrap utility classes like "p-1" (see: https://getbootstrap.com/docs/5.0/utilities/spacing/)

Or grid/flex layout can be solely described via Bootstrap's utility classes. There was often no need to create custom CSS.

[EliasMasche]

I started making some changes to Bootstrap. What is the plan?

Besides, in this commit based on bootstrap, there are two ways to keep the custom class or use an inline. Checking through the repo, this is the only instance, so I guess using an inline CSS is fine
10e8472 (#413)

And I am already triggering some fails in tests because the HTML classes are missing in the razor files. For the moment, I am working on replacing them with Bootstrap but keeping the custom classes in basics.css for fallback. I will compare if the appearance is working as well as the old one, and once I am done, I will remove those custom classes.

[linkdotnet]

I started making some changes to Bootstrap. What is the plan?

I am not sure if I understand correct? What do you mean by that?

Besides, in this commit based on bootstrap, there are two ways to keep the custom class or use an inline. Checking through the repo, this is the only instance, so I guess using an inline CSS is fine
10e8472 (#413

If we keep stuff then still as classes. That is fine. If we see a chance to remove the custom CSS, that would be great, if not - then let's keep it as it is.

For the moment, I am working on replacing them with Bootstrap but keeping the custom classes in basics.css for fallback

Why would we need them as fallback? Sure, the tests have to be adopted and that is unfortunately a bit of a shortcoming on the current test-setup that it uses too much the concrete CSS classes. But that is a whole different story :D so for now it might be the best, to change failing ones to the new classes to it still targets the same object.

[EliasMasche]

I am not sure if I understand correct? What do you mean by that?

You replied this question in the second, I was asking or trying to ask if I should refactor until it is done or not possible to do it.

Why would we need them as fallback? Sure, the tests have to be adopted and that is unfortunately a bit of a shortcoming on the current test-setup that it uses too much the concrete CSS classes. But that is a whole different story :D so for now it might be the best, to change failing ones to the new classes to it still targets the same object.

Sorry, what I mean is I still keep the old custom CSS classes for the time being for testing if the new HTML bootstrap CSS classes are working properly. Once I finish, I will do the cleaning. I am keeping a list of the custom CSS classes and commenting as I progress with the refactor

[EliasMasche]

I did some progress overall, trying to match or mimic the appearance from custom CSS classes with Bootstrap 5 and with that able to reduce LoS in basic.css, the .blog-card class still not able to achieve progress or at least fully replace it but did at minor places, the rest took time.

There is this new warning:

Error: /Users/runner/work/Blog/Blog/src/LinkDotNet.Blog.Web/Features/Home/Components/IntroductionCard.razor(42,20): error S1144: Remove the unused private property 'IntroductionStyle'. (https://rules.sonarsource.com/csharp/RSPEC-1144) [/Users/runner/work/Blog/Blog/src/LinkDotNet.Blog.Web/LinkDotNet.Blog.Web.csproj]

• Is it free to remove safely without any collateral damage?
• Is it fine that I check the rest of the HTML classes based on Bootstrap 4 and update them to 5? I think I found some that are using the semantics of 4

About the tests, how is gonna be the progress going to be?