Skip to content

Redirects: match recent improvements #237

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

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Dec 6, 2023

Matches readthedocs/readthedocs.org#10881.

Applied the same code we have for automation rules.

redirects

There are some new options that we may want to include in the UI, like status code, description, and if the redirect is enabled/disabled. We can also think about implementing some nice drag and drop in addition to the arrow buttons.

Matches readthedocs/readthedocs.org#10881.

Applied the same code we have for automation rules.
@stsewd stsewd requested a review from a team as a code owner December 6, 2023 22:02
@stsewd stsewd requested a review from agjohnson December 6, 2023 22:02
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for jumping in on this change 💪

@agjohnson
Copy link
Contributor

There are some new options that we may want to include in the UI, like status code, description, and if the redirect is enabled/disabled.

The listing UIs do tend to get overwhelmed with too much data in each listing item, but there are a few options. For status code or similar, I think a listing filter works really well. I'd say a listing filter would at least have redirect type and status code, maybe there are other fields. We don't have to list this in each item for the UI/UX to make sense here, as users can enable the filters to see the matching rules.

Filtering does complicate the UI around rule ordering though, for both the arrows and the drag and drop. We can always disable these methods if the filter is active though.

We can also think about implementing some nice drag and drop in addition to the arrow buttons.

I would aim for implementing explicit ordering first and allow users to just type in a priority number or something. Drag and drop doesn't work great here because this list will eventually paginate. It's a nice UX affordance for later maybe though.

Feel free to move these to a dedicated issue and we can come back later

@stsewd
Copy link
Member Author

stsewd commented Dec 7, 2023

@agjohnson opened #241.

Another thing, I'm not sure about using the $1/$lang/etc placeholders. It may confuse users about that syntax being supported, when it's not.

Specially for types like clean url/html url, they are displayed like /**/$1.html -> /**/$1/. What about showing the description of this redirect instead? Clean URL to HTML, and with an example <file>/ to <file>.html

We are also using <placeholder> in our docs, maybe we can re-use that here.
This is, /<lang>/<version>/.

@agjohnson
Copy link
Contributor

I don't like <version> as it's not a string interpolation format used by any language I know of, and it looks vaguely like HTML. Given our primary audience is Python developers, I'd actually go for {version} before any other pseudo variable formats, though if $version isn't clear enough, I don't think any of these will be clear to the user.

I might just say that instead of switching between pseudo variable formats, the styles should be more specific, and should stand out more. I can play around with the element structure here. I'm guessing I'll end up using .ui.label inline in the URL.

We should absolutely replace $1 with something more verbose either way.

Do we have any other variables we need to describe in the UI besides:

  • project
  • version
  • filename

@stsewd
Copy link
Member Author

stsewd commented Dec 7, 2023

Do we have any other variables we need to describe in the UI besides:

We have project prefix, subproject prefix, and 3 types of versioning scheme which will result in a different prefix for page redirects... but anything that describes page redirects applying to only the page/filename of any version would be ok.

@agjohnson
Copy link
Contributor

agjohnson commented Dec 7, 2023

Okay, maybe page instead of filename makes the most sense then.

:splat is a weird one. I'm not sure how to represent that in the URL with a verbose name. The name "splat" is probably not understood outside quite technical circles, and even then probably doesn't communicate to the user that this is will match and capture any characters. I'd have to think more here.

Overall, the main thing I would want to preserve in this UI is that the redirects are not listed by their verbose name ("Clean URL to HTML", etc). These names make comparing the redirects quite difficult. I'll want to represent all redirects as from/target.

I think what you've done is pretty close for now though, it's only really the :splat in the target URL that will confuse users. Tweaking the display visually is something that we can iterate on later.

@stsewd stsewd merged commit bff4b4d into main Jan 2, 2024
@stsewd stsewd deleted the redirects-improvements branch January 2, 2024 19:27
@@ -37,6 +37,34 @@
{% endblock list_placeholder_text %}

{% block list_item_right_buttons %}
{% trans "Move up" as button_up_text %}

This comment was marked as spam.

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

Successfully merging this pull request may close these issues.

3 participants