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 CSS variables controlling column widths in the "table" view #126

Conversation

IMSoP
Copy link
Contributor

@IMSoP IMSoP commented Feb 10, 2023

Depending on how deeply nested your schema is, and how long your property names, you might want to devote more (or less) width to the "key" column in the "table" view.

The easiest way to make this configurable seems to be with CSS variables, defaulting to the original width.

Depending on how deeply nested your schema is, and how long your
property names, you might want to devote more (or less) width to
the "key" column in the "table" view.

The easiest way to make this configurable seems to be with CSS
variables, defaulting to the original width.
@@ -16,6 +16,7 @@ The available variables are:
Explorer location | Variables
--------------------|--------------------
Fonts | `--font-regular` `--font-mono`, `--font-size-small`, `--font-size-regular`, `--font-size-mono`
Schema Table View | `--schema-table-key-width`, `--schema-table-type-width`
Copy link
Member

Choose a reason for hiding this comment

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

Can we take out the -table part? Reason being that keeping the number of variables at a minimum is a huge plus, which let's us reuse this effectively in schema tree, if necessary.

But I want to have a slightly longer discussion about if there is an alternative to getting this right. I can imagine it won't be easy/obvious for someone to change these values so that they are happy no matter what. I'm wondering if there is something else we can do.

For my understanding, can you share an example schema that contains this issue so we can figure out what alternatives could make sense here? I would love it if this could be automatically handled, assuming we wrote some code like that, any ideas on what that would be? An example would do wonders for pondering that answer.

@IMSoP
Copy link
Contributor Author

IMSoP commented Feb 13, 2023

Here's an example with a few levels of nesting that demonstrates the problem: https://gist.github.com/IMSoP/693a511bf205d282c59c0929f1fe4222

Current behaviour:

1--nested-schema-current-behaviour

Proposed, with

openapi-explorer {
    --schema-table-key-width: 300px;
    --schema-table-type-width: 100px;
}

2--nested-schema-forced-width


The fundamental problem is that this "table" isn't actually using an HTML table, so the column can't expand to fit the content. I'm not sure if there's a reason behind that choice of markup, but even if it was changed, we'd probably want some kind of min-width and max-width, and those would again depend on the schema.

In some ways, the ideal would be to have that column fixed width, but with a horizontal scroll-bar, since most of the space is taken up by the indenting. I've no idea how to achieve that in CSS, though, regardless of the HTML markup.


The tree view is currently laid out in a completely different way: the labels are all inline elements, with the whitespace managed by padding on nested containers. The result is that wide elements just push the type and description to the right:

3--nested-schema-tree-view

As far as I can make out, the width is managed by an inline min-width calculated for each level in the JS; I think it's this code:

const minFieldColWidth = 300 - (indentLevel * leftPadding);

I'm not sure if that can be re-implemented in CSS, or if the whole markup would need to be reworked.

As a quick fix, a small margin-right on the type would stop the text colliding like it does right now.

@wparad
Copy link
Member

wparad commented Feb 13, 2023

That's some good insight, I'm curious how the max nesting width is currently is affected if the text isn't wrapped (i.e. if we removed that limitation altogether. Let me see if I can play around here and come up with a more automated solution.

@wparad
Copy link
Member

wparad commented Feb 15, 2023

The fundamental problem is that this "table" isn't actually using an HTML table, so the column can't expand to fit the content. I'm not sure if there's a reason behind that choice of markup, but even if it was changed, we'd probably want some kind of min-width and max-width, and those would again depend on the schema.

It can't be a <table> because as far as anyone knows there is no way to dynamically collapse it using a transition, which would break the ability the have "collapsable" nesting. (I just tried this, it doesn't work, can't find any working solution here without a dynamic table).

In some ways, the ideal would be to have that column fixed width, but with a horizontal scroll-bar, since most of the space is taken up by the indenting. I've no idea how to achieve that in CSS, though, regardless of the HTML markup.

Could you say more about this?

Copy link
Member

@wparad wparad left a comment

Choose a reason for hiding this comment

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

I'm still pondering on what's the best solution for this. I've got some bad ideas for this, but nothing I like yet. I think the solution is going to be go with a dynamic calculation of the width of displayed deeply nested cells, calculate a predictive width and then set the column to be that with some extra right side padding. Then we won't need these extra parameters and it will work for everyone directly out of the box without needing to configure these magic vars.

Why? Because it isn't really optional, if feels like a bug, and everyone that has this edge case will hit this problem, which means we need a full solution.

@wparad
Copy link
Member

wparad commented Jul 6, 2023

Closing due to lack of complete solution and no recent updates to the PR. #229

@wparad wparad closed this Jul 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
@wparad
Copy link
Member

wparad commented Feb 7, 2024

FYI, an optimized version of this has actually be recently released in, that does automatic column resizing.

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

Successfully merging this pull request may close these issues.

2 participants