-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SPARK-46923][DOCS] Limit width of configuration tables #44955
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
it overrides the plain `code` styling because of its specificity and doesn't really help
This reverts commit 9ee72be.
HyukjinKwon
approved these changes
Jan 31, 2024
Merged to master. |
For a demonstration of how we can use this technique to improve the presentation of config tables once we are programmatically generating them, refer to #44971. |
This was referenced Feb 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
spark-config
.docs/sql-ref-ansi-compliance.md
from Markdown to HTML and assign it to this new CSS class as well.spark.worker.resourcesFile
that is not needed anymore thanks to these changes..global
CSS rules that, due to their specificity interfere with our ability to assign simple rules that apply directly to elements.Why are the changes needed?
Many configs and config defaults have very long names that normally cannot wrap. This causes tables to overflow the viewport. An egregious example of this is
spark.scheduler.listenerbus.eventqueue.executorManagement.capacity
, which has a default ofspark.scheduler.listenerbus.eventqueue.capacity
.This change will force these long strings to break and wrap, which will keep the table widths limited to the width of the overall content. Because we are hard-coding the column widths, some tables will look slightly worse with this new layout due to extra whitespace. I couldn't figure out a practical way to prevent that while also solving the main problem of table overflow.
In #44755 or #44756 (whichever approach gets accepted), these config tables will be generated automatically. This will give us the opportunity to improve the styling further by setting the column width dynamically based on the content. (This should be possible in CSS, but table styling in CSS is limited and we cannot use properties like
max-width
.) We will also be able to insert word break opportunities so that config names wrap in a more visually pleasing manner.Does this PR introduce any user-facing change?
Yes, it changes the presentation of tables, especially config tables, in the main documentation.
How was this patch tested?
I built the docs and compared them visually across
master
(left) and this branch (right).sql-ref-ansi-compliance.html
:configuration.html#scheduling
:configuration.html#barrier-execution-mode
:spark-standalone.html
:structured-streaming-kafka-integration.html#configuration
:Was this patch authored or co-authored using generative AI tooling?
No.