readthedocs / ext-theme

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

Add basic attempt at Resync verisons button #481

Closed ericholscher closed 1 month ago

ericholscher commented 2 months ago

This is a bit hacky but works for me locally. Could use some UI/UX feedback to make it work better, I mostly just copied what was being used in other places.

Fixes https://github.com/readthedocs/ext-theme/issues/65

Screenshot 2024-09-11 at 3 00 26 PM

ericholscher commented 2 months ago

I've updated this to put it in the activate a new version page. It basically just spins for a second, and then goes back to the same button. I removed the reload since the UI here is using an API-driven addition workflow, so it should update automatically without a reload.

Not sure the best pattern here -- would be nice actually to do this inline and have a way to do a notification when the sync is finished with results, but this seems like a reasonable first step?

Screenshot 2024-09-18 at 2 31 07 PM

agjohnson commented 2 months ago

I would reserve that placeholder space for onboarding content. All other views use this space to point the user to more information in our docs/etc, and the button feels quite prominent for what it's function is -- manually resyncing is great to have in our UI but it is still an outlier in UX.

I'd instead reuse the existing pattern from the repository listing I pointed out above

image

https://github.com/readthedocs/ext-theme/blob/8239442a87d657aa0ac82aff9de92a73e556c838/readthedocsext/theme/templates/projects/partials/project_create_automatic.html#L101-L112

You'd use something like:

<div class="ui small bottom attached center aligned message">
  {% trans "Are your repository branches out of date?" %}
  <readthedocs-button-sync-versions>
    <button class="ui mini black basic compact right aligned button"> 
      {% trans "Resync repository" %}
    </button>
  </readthedocs-button-sync-versions>
</div>
ericholscher commented 1 month ago

Something like this:

Screenshot 2024-09-24 at 2 19 18 PM
agjohnson commented 1 month ago

That's pretty close, but looks like you added the element inside the .ui.placeholder.segment still. I'd check out that example from above, it shows the structure of the .ui.bottom.attached.message, but also the preceding elements:

https://github.com/readthedocs/ext-theme/blob/8239442a87d657aa0ac82aff9de92a73e556c838/readthedocsext/theme/templates/projects/partials/project_create_automatic.html#L101-L112

In that, the .ui.bottom.attached.message is sibling to the outer .ui.basic.segment > .ui.placeholder.segment, but the .ui.basic.segment is only for hiding this placeholder block anyways.

ericholscher commented 1 month ago

This class structure is super finicky... I don't feel like I'm really learning anything about why it should actually work this way, and it's just futzing with stuff without any real direction. Is there any context for this structure?

agjohnson commented 1 month ago

The example I've linked to had the structure we want to replicate and copy. It uses two divs, summarized as:

<div class="ui top attached segment"></div>
<div class="ui bottom attached segment"></div>

This gives two attached segments -- these have no spacing between them and they have a border separating them.

You can read about this here:

https://fomantic-ui.com/elements/segment.html#attached

Your last attempt was, summarized:

<div class="ui top attached segment">
  <div class="ui placeholder segment">
    <div class="ui bottom attached segment">
    </div>
  </div>
</div>

This will render the bottom attached segment inside the placeholder segment, which has its own separate padding and border. The two attached segments need to be sibling elements instead.

Placeholder segments are described here:

https://fomantic-ui.com/elements/segment.html#placeholder-segment

agjohnson commented 1 month ago

What was here was close, but needed e9a17d8

With that, it renders correctly, and matches the repository listing UI:

image

ericholscher commented 1 month ago

The example I've linked to had the structure we want to replicate and copy. It uses two divs, summarized as:

<div class="ui top attached segment"></div>
<div class="ui bottom attached segment"></div>

This gives two attached segments -- these have no spacing between them and they have a border separating them.

You can read about this here:

https://fomantic-ui.com/elements/segment.html#attached

Your last attempt was, summarized:

<div class="ui top attached segment">
  <div class="ui placeholder segment">
    <div class="ui bottom attached segment">
    </div>
  </div>
</div>

This will render the bottom attached segment inside the placeholder segment, which has its own separate padding and border. The two attached segments need to be sibling elements instead.

Placeholder segments are described here:

https://fomantic-ui.com/elements/segment.html#placeholder-segment

Nice -- we should drop this in the frontend docs somewhere, it's useful context 💯

humitos commented 1 month ago

I think we should add some kind of feedback or message to the user once you press the button. It shows the spinner and it stops immediately. I'd say that we should probably put a time for 2 or 3 seconds, and add a message saying "Synced, your version should appear in the list now" or similar.

Peek 2024-10-09 16-23