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

add styling for dl, dt, dd elements #19034

Closed
wants to merge 1 commit into from

Conversation

kbhawkey
Copy link
Contributor

@kbhawkey kbhawkey commented Feb 8, 2020

Testing styling of definition lists.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 8, 2020
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Feb 8, 2020
@netlify
Copy link

netlify bot commented Feb 8, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 7942bd5

https://deploy-preview-19034--kubernetes-io-master-staging.netlify.com

@kbhawkey
Copy link
Contributor Author

kbhawkey commented Feb 8, 2020

@sftim
Copy link
Contributor

sftim commented Feb 9, 2020

I'm overall fine with the idea of updating the styling here.

The style guide recommends using italics when defining or introducing new terms so, with that in mind, how about making the text contents of <dt /> elements be italic?

@kbhawkey what do you think? If you want to ship this as-is, you can /unhold

/hold
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Feb 9, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 9, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2020
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@kbhawkey
Copy link
Contributor Author

@sftim, I am not sure if the italics work in this case. The block style seems to be closer to the list format and the style of the glossary.
@Colstuwjx , What are your thoughts on the styling and the look of the definition list in the Ingress page?

@zacharysarah
Copy link
Contributor

zacharysarah commented Feb 10, 2020

@kbhawkey

I am not sure if the italics work in this case. The block style seems to be closer to the list format and the style of the glossary.

I think following the glossary example is fine here, but in that case I would bullet the terms if possible. (Will this impact #18968?)

This is a much cleaner format--thanks for the improvement! ✨

@kbhawkey
Copy link
Contributor Author

kbhawkey commented Feb 11, 2020

@kbhawkey

I am not sure if the italics work in this case. The block style seems to be closer to the list format and the style of the glossary.

I think following the glossary example is fine here, but in that case I would bullet the terms if possible. (Will this impact #18968?)

This is a much cleaner format--thanks for the improvement! ✨

@zacharysarah , The glossary is created from an unordered list and has some custom styling.
I don't think I can add a styled bullet to a definition list term. I created this PR to evaluate whether
a styled definition list is an improvement over the basic unordered list. Yes, PR #18968 is impacted.

@kbhawkey
Copy link
Contributor Author

Closing this issue, unless I hear that the docs need styling for definition lists 😃 .
/close

@k8s-ci-robot
Copy link
Contributor

@kbhawkey: Closed this PR.

In response to this:

Closing this issue, unless I hear that the docs need styling for definition lists 😃 .
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kbhawkey kbhawkey deleted the kb-styling-dl branch February 18, 2020 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants