Skip to content

use CSS instead of inlining styles 🖌️ #181

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.

Sign up for GitHub

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

Jump to bottom
Merged
jamacku merged 1 commit into from
May 15, 2024
Merged

use CSS instead of inlining styles 🖌️ #181

jamacku merged 1 commit into from
May 15, 2024

Conversation

@jamacku
Copy link
Member

@jamacku jamacku commented May 10, 2024

The branch contains additional commits from:

Once they are accepted, I'll drop them.

This is just a DRAFT. There is probably a better way how to implement CSS into writer-html

@jamacku
Copy link
Member Author

jamacku commented May 10, 2024

Related to #174 ?

ferdnyc
ferdnyc reviewed May 11, 2024
View reviewed changes
Copy link

@ferdnyc ferdnyc left a comment

Choose a reason for hiding this comment

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

This definitely looks like a HUGE step in the right direction, @jamacku. Thanks for looking at it!

kdudka
kdudka reviewed May 13, 2024
View reviewed changes
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

@jamacku Looks good! A few suggestions inline...

jamacku added a commit to jamacku/csdiff that referenced this pull request May 14, 2024
@jamacku
feat(cshtml): use CSS instead of inlining styles
5f3b933 
Fixes: csutils#174
Closes: csutils#181
@jamacku jamacku self-assigned this May 14, 2024
@jamacku
Copy link
Member Author

jamacku commented May 14, 2024

@kdudka, @ferdnyc, thanks for the review. I have addressed your comments in 5f3b933.

@jamacku jamacku marked this pull request as ready for review May 14, 2024 08:37
@jamacku jamacku requested a review from kdudka May 14, 2024 08:39
lzaoral
lzaoral reviewed May 14, 2024
View reviewed changes
@jamacku jamacku requested a review from lzaoral May 14, 2024 12:48
kdudka
kdudka reviewed May 14, 2024
View reviewed changes
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

@jamacku Thanks for the update! Two more remarks inline...

@jamacku jamacku requested a review from kdudka May 15, 2024 07:08
kdudka
kdudka approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

@jamacku Looks good. Thanks for the update!

@jamacku jamacku merged commit a8f67f1 into csutils:main May 15, 2024
@jamacku jamacku deleted the css branch May 15, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kdudka kdudka kdudka approved these changes

@lzaoral lzaoral lzaoral approved these changes

+1 more reviewer

@ferdnyc ferdnyc ferdnyc left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jamacku jamacku

Labels

enhancement html

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jamacku @kdudka @ferdnyc @lzaoral