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

Test and cleanup the disabled <link> node logic #543

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

MatthewHerbst
Copy link
Owner

Follow up to #537

@luckrnx09 Let me know what you think!

@MatthewHerbst MatthewHerbst force-pushed the test-and-cleanup-disabled-link-node branch from 0e02c7c to 892341c Compare November 2, 2022 08:02
@luckrnx09
Copy link
Contributor

@MatthewHerbst Thank you for your careful consideration. I need to spend some time to study whether it can be filtered through the selector. I think I will reply to you tomorrow

@luckrnx09
Copy link
Contributor

luckrnx09 commented Nov 3, 2022

hi @MatthewHerbst , sorry for reply you so late.
Almost looks good for me, but two ideas about this PR:

  1. Prefer add a rel="stylesheet" for each link tags you added in this PR, because we are importing the css files from outside.
  2. We can use a selector link["stylesheet"]:not([disabled]) to filter links which has the disabled attribute, because the css will not be applied if the link has attribute disabled="false".
    See: https://codesandbox.io/s/zealous-http-0g0p4c?file=/index.html

Correct me If I am wrong

@MatthewHerbst
Copy link
Owner Author

I haven't been able to get the selector to work for some reason. We can revisit it in the future if we figure it out. For now I think this is good to go, and I made some improvements to the examples (tests 😉 ) as well

@MatthewHerbst MatthewHerbst merged commit 579d661 into master Nov 3, 2022
@MatthewHerbst MatthewHerbst deleted the test-and-cleanup-disabled-link-node branch November 3, 2022 18:33
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.

2 participants