readthedocs / ext-theme

Read the Docs drop in replacement site templates
2 stars 2 forks source link

Version creation and update form updates #337

Closed agjohnson closed 5 months ago

agjohnson commented 5 months ago

Fixes:

humitos commented 5 months ago

Note that when using "Fixes:" with bullets, those issues are not closed automatically when the PR is merged, unfortunately 😞

agjohnson commented 5 months ago

Also, I'd like to align the "Active" element to the right

The reason I am not doing this is I am matching the project create search view. I decided we should not make a new pattern here.

image

There is an image to the right, we don't have an image for versions however.

I'd prefer to keep the order "icon + verbose name".

The reason I am not doing this is to match the project create search. Also because icon then title pushes the header over to the right, so the description portion is then too far left. This then also needs an icon to push the text right and there also isn't an icon that captures version.identifier -- which can be a commit, tag, or branch.

On the version detail page, I originally did this and it looked too noisy and cluttered. I reflected the change here, and this matches patterns used through this UI better now.

The UI did look like this:

image

The icons usually match each other and it so it looks redundant. We don't use an icon in the header like this anywhere else in the UI also, so I'm not too tied to even keeping it here. I matched this to the search results view after (top line title is verbose name + icon, description line is a code element with the identifier -- again no icon here because there is no explicit description of what the identifier is.

This matches how we show the "Hidden" and "Default" version on the list of versions.

Indeed, but those are listing views. I'm aiming to consolidate the search results views for now.

humitos commented 5 months ago

There is an image to the right, we don't have an image for versions however.

Gotcha. Can we put the "Active" label in the middle of the row instead so it's aligned between all the versions as shown in the last screenshot of https://github.com/readthedocs/ext-theme/pull/337#pullrequestreview-2014445589? I think it looks lot better.

Also because icon then title pushes the header over to the right, so the description portion is then too far left

Also note that header and description won't match anyways, since use different fonts (the one from the description is monospaced which make the text a little longer)

agjohnson commented 5 months ago

Gotcha. Can we put the "Active" label in the middle of the row

Not without headaches, there isn't enough horizontal room for a floating element like this. Project names that aren't short will cause this floating element to appear in odd places -- either push it out of column alignment or push it on to a new line. Really, you need a table for what you want and there isn't enough room.

Placed where it is now, it's intentionally staggered vertically and will look better than when vertical alignment randomly fails.

Also note that header and description won't match anyways, since use different fonts (the one from the description is monospaced which make the text a little longer)

Yeah, longer is fine and expected, it's the unaligned start of the text that is the larger issue. When text doesn't align anywhere is when it looks awkward.

agjohnson commented 5 months ago

Also, perhaps another pattern that I might prefer here is expanding labels instead of text labels:

I would want to play around with this as an option here, though would be careful that an icon label was enough to signal to the user that the version is already active -- reducing the text to an icon might not be as clear.

humitos commented 5 months ago

Looks great! However, it seems pretty low priority to me right now. I'd leave it for when we are polishing the dashboard in the future. I'm fine with what you've done for now.

agjohnson commented 5 months ago

I was not planning on it, no. This is a feature addition for later.