Skip to content

Improve third-party repo UX#2480

Open
Eauldane wants to merge 5 commits into
goatcorp:masterfrom
Eauldane:tpr-ux
Open

Improve third-party repo UX#2480
Eauldane wants to merge 5 commits into
goatcorp:masterfrom
Eauldane:tpr-ux

Conversation

@Eauldane

Copy link
Copy Markdown

A surprisingly common complaint I keep coming across is that people have trouble adding third-party repos. The current workflow requires pressing the "plus" button to add an empty row after pasting the last URL you want to add in. This PR changes the behaviour so that a user can simply paste in the URL and hit the save button without the unnecesary row add.

Some changes to the saving logic were needed to accomodate this, since validity checks were tied to the add button - the save button would otherwise close the window, hiding the error message if there is one. The window will stay open now if there's an error, or close as before if everything is good.

I've tested this with changing both the repo list and adding dev DLLs and it appears to work as intended.

@Eauldane Eauldane requested a review from a team as a code owner November 30, 2025 20:28
this.thirdRepoTempUrl = this.thirdRepoTempUrl.Trim();
if (this.thirdRepoList.Any(r => string.Equals(r.Url, this.thirdRepoTempUrl, StringComparison.InvariantCultureIgnoreCase)))
{
this.thirdRepoAddError = Loc.Localize("DalamudThirdRepoExists", "Repo already exists.");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason you are clearing the errors after a while? Seems unintuitive, since the input is still not valid after they are cleared, but there is no more feedback

ImGui.NextColumn();
ImGui.SetNextItemWidth(-1);
ImGui.InputText("##thirdRepoUrlInput"u8, ref this.thirdRepoTempUrl, 300);
if (ImGui.InputText("##thirdRepoUrlInput"u8, ref this.thirdRepoTempUrl, 300))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it not make more sense to run the validations every time this input changes? Then you don't have to add the double-check SettingsWindow and people still get immediate feedback. Just have Save() write the new repo automatically.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fair point - I usually work on systems where not waiting for user confirmation before validation is frowned upon due to resource constraints or convention, so I was still in that mindset when I wrote that. Fixed in the next commit!

@Eauldane

Eauldane commented Dec 3, 2025

Copy link
Copy Markdown
Author

Requested changes have been made - errors don't clear after a few seconds now, and input is validated as it happens. The latter led me to discover that the existing validation would consider an entry of just http:// or https:// to be valid, so I took the liberty of adjusting that to require at least a host be set as well. This could be extended to actually poll the URL and check for the content type being JSON, but that feels out of scope for this PR.

@Eauldane Eauldane requested a review from goaaats December 3, 2025 22:22
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.

2 participants