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

New article for Blazor CSS isolation #19956

Merged
merged 15 commits into from Oct 22, 2020

Conversation

daveabrock
Copy link
Contributor

Here's the new article on Blazor CSS isolation. I'm looking forward to your feedback.

I struggled a little with the CSS preprocessor section and how much in detail we should go into it. After all, CSS isolation works out of the box well, so don't want to overwhelm, but there's also a lot of folks that are interested in this.

cc @guardrex @javiercn

Fixes #19360

@guardrex
Copy link
Collaborator

We haven't forgotten about this @daveabrock ... we should be getting to it very soon.

@daveabrock
Copy link
Contributor Author

@javiercn and @guardrex - updated the docs based on @javiercn and @meziantou feedback. If that is all good, @guardrex might be able to take a look now...

@guardrex
Copy link
Collaborator

guardrex commented Oct 2, 2020

I'll wait for @javiercn to sign off first ... then make a pass and ping u for a final look.

@guardrex guardrex requested review from captainsafia and removed request for javiercn October 7, 2020 09:40
@guardrex
Copy link
Collaborator

guardrex commented Oct 7, 2020

@captainsafia ... Can you perform the technical review and then I'll round up the UE updates?

aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved
@daveabrock
Copy link
Contributor Author

Pushed changes based on recent feedback (thanks @captainsafia). cc @guardrex

@guardrex
Copy link
Collaborator

guardrex commented Oct 9, 2020

Cool ... I'll make an edit pass asap, but it might be early next week. I'm 🏃🏃🏃😅 on Blazor security work at the moment.

@danroth27
Copy link
Member

@daveabrock @guardrex FYI - there are various changes coming to CSS isolation in .NET 5 RC2 that we'll need to react to in this doc: dotnet/aspnetcore#25565

@guardrex
Copy link
Collaborator

guardrex commented Oct 9, 2020

@daveabrock ... I'll hold off on my pass until you get a chance to react; or if you're too busy for the updates, I'll take a shot at getting them into the doc.

@daveabrock
Copy link
Contributor Author

@daveabrock ... I'll hold off on my pass until you get a chance to react; or if you're too busy for the updates, I'll take a shot at getting them into the doc.

I can take a look at these. I'll do some updates this weekend, then test the updates when RC2 comes out. Which I'm sure is very soon. 😁

@daveabrock
Copy link
Contributor Author

With RC2 out now, @guardrex, should have updates in tonight and updates ready for review tomorrow AM.

@guardrex
Copy link
Collaborator

Sure thing ... no worries ... we're still fine here. We target to make sure that docs are up by release at the latest, then we go for preview when we're able. I certainly wouldn't rush you on updates given that you're putting in your time and effort on this. It's great to have your help. I'm still getting the last of the .NET Blazor WASM security updates in, so you've taken a real burden off of me so that I can focus on these security updates. If you even need a little more time, don't sweat it ... take the time. We still have a few weeks left to go.

@daveabrock
Copy link
Contributor Author

OK, @guardrex, I pushed updates for RC2. I created a Razor Class Library in the sample app to understand the changes and how projects are referenced—slick!

A subtle change is in the first note, I added the "Razor" word:
"In order to guarantee style isolation when bundling occurs, @import Razor blocks are not supported with scoped CSS files."

I clarified this because the generated bundles use CSS imports but this is still not allowed:

@css { }

Adding "Razor" to the note probably avoids further confusion.

@guardrex
Copy link
Collaborator

Thanks @daveabrock ... I'll be free I think on ...... errrrrrr 🤔 ..... Monday late morning. My Friday is already full, and I have a priority WASM hosting regression to address Monday morning.

@daveabrock
Copy link
Contributor Author

@guardrex - sounds great! (The review, not the regression. 😉)

@guardrex
Copy link
Collaborator

guardrex commented Oct 19, 2020

@daveabrock ... Almost here! 🏃😅

It only took TEN hours today to get my hosted WASM with AAD groups and roles security code running. Yikes! 😄 Well .... at least it IS running! 🎉🍻

This PR is coming up next. I reached a 🧠🔥 brain fry 🧠🔥 about an hour ago (and I need to get the Blazor 5.0 bits into the What's New for 5.0 topic tonight), so I won't get to this by EOD. Hopefully, I'll reach this by tomorrow (Tuesday).

@daveabrock
Copy link
Contributor Author

It only took TEN hours today to get my hosted WASM with AAD groups and roles security code running. Yikes! 😄 Well .... at least it IS running! 🎉🍻

Thoughts and prayers. I really appreciate you keeping me up to date!

@guardrex
Copy link
Collaborator

Sure thing. I don't think they'll be much more for you to do. I'll make a direct edit pass that will probably consist of nit updates for our style guidelines. I'll ping you to review the updates, then we'll ask the product unit if it's ok to get this content merged.

@guardrex
Copy link
Collaborator

guardrex commented Oct 20, 2020

Great job ... I ❤️ the doc!

You might (er ... likely will) need to edit my changes a bit. I may have 💥:cry: a bit of meaning that you'll need to correct.

Notes on my changes:

  • I use placeholders in the format {PLACEHOLDER} with text that explains what the placeholder is. This is because localization doesn't happen on code. Therefore, we have a better chance that non-English readers will understand the docs.
  • Your Parent-Child example ... I changed it to "Parent" and "Child" because having the word "child" in the name of the parent component didn't seem helpful and may lead to confusion. I also took off the @page directive from the child as unnecessary. See if that example holds up with my changes. If not, change it, but try not to use "child" in the parent component's name.
  • There's a friendly disagreement among doc authors on this repo about prepositional phrases and commas in openers. IMO, a prepositional phrase that modifies an object of another prepositional phrase is almost always an essential phrase; therefore, I personally almost never comma them. Example: "For example to eat the cake, I grabbed a fork." Authors here feel that the second phrase is non-essential in nature and will set it off with commas ("For example, to eat the cake, I grabbed a fork."). I don't think that's correct. IMO, the 2nd phrase exactly specifies the word "example" and thus is necessary/essential. That's almost always the case with a prepositional phrase that follows another prepositional phrase. Anyway ... to solve it and make everyone happy, I just try to avoid the construction. [btw- This is also true with adverbs and prepositional phrases. Example: "Additionally to eat the cake, I use a fork." (no comma)] You'll see some updates on the commit along these lines. Often, "for example" can just be eliminated as unnecessary. Other times, the sentence can be changed to avoid double-prepositional phrase openers. .......... Ok .... 😄 ... Professor Luke's English 101 course is now OVER! 😁 lol

If you find that you're getting too many pings from being in the author metadata after this is published, we can assign it to me for the pings. That might save your Inbox a bit if you're getting too many hits from readers. However, it isn't a bad idea to leave yourself there for a while going into early next year so that you can interact immediately with anyone who opens an issue on the topic. I leave this aspect up to you. You can send a PR anytime you want to change it.

After you fix 🙈 any of my edits that went sideways, we'll see about wrapping this up and getting it merged.

@daveabrock
Copy link
Contributor Author

daveabrock commented Oct 20, 2020

Actually, I went through this when I was emailed about your push and I think it looks good, and you haven't lost any meaning with your changes.

With this and my blog post being the top search result for this topic, for better or worse I am the "Blazor CSS isolation guy" so I may as well embrace it! We can leave my name on, and I'll ping you if it gets to be a little too much.

@guardrex
Copy link
Collaborator

"I may as well embrace it!" ..... put that on my tombstone please. ⚰️ lol

If you end up with too many pings and having to deal with too many dev support requests, just send a PR changing the author metadata to guardrex and leave the author byline alone. You always keep the credit ... I'll take the HITS! 👦🤛😵 ouch!

@captainsafia ... Do you want to take a final look and then I can merge this, or do you want to call for additional product unit review before merging? I suspect that all of the GA coverage will be reviewed in the live preview topics in the days leading up to release or very soon thereafter, so I think we can go ahead and merge this.

aspnetcore/blazor/components/css-isolation.md Outdated Show resolved Hide resolved

In the preceding example, the CSS generated for `MyComponent.Razor.css` changes its scope identifier from `b-<10-character-string>` to `my-custom-scope-identifier`.

### Change base path for static web assets
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right place to include this heading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, there's no natural way unless we include it in the main section where it's discussed. I personally wanted to save the config to the end as it is optional and probably rarely used.

Co-authored-by: Safia Abdalla <safia@safia.rocks>
@guardrex
Copy link
Collaborator

btw @daveabrock ... For that Line 24 change ... "along side" 👉 "alongside" (one word)

@daveabrock
Copy link
Contributor Author

🚢?!?!?!

@guardrex
Copy link
Collaborator

guardrex commented Oct 21, 2020

🚢?!?!?

We'll wait for @captainsafia to give a final okey-dokey and say if we should call anyone else in. Personally, I think it's good to go. We can have additional 👀 anytime up to and just after 5.0 GA on the live topic for last minute nits.

@javiercn
Copy link
Member

I'll do a pass on this tomorrow

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

@guardrex
Copy link
Collaborator

Thank U so much @daveabrock for taking this on. Your topic contribution is 🥇 greatly appreciated! 🙇‍♂️.

After merging this PR, I'll do a live docs update, so this should be online shortly in the preview docs.

@guardrex guardrex merged commit da0f030 into dotnet:master Oct 22, 2020
@voroninp
Copy link
Contributor

Still cannot locate sections about minification.

@guardrex
Copy link
Collaborator

There's nothing built-in for that, @voroninp. You can use any 3rd party thing that you like. Check with the community on it for ideas on what folks are using ...

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

Successfully merging this pull request may close these issues.

[RC1] Blazor CSS isolation
7 participants