Skip to content

Improve third-party repo UX #2480

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.

Sign up for GitHub

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

Open
Eauldane wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Improve third-party repo UX #2480

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
efa8f06
Allow saving third party repos without unnecesary "plus" press
Eauldane Nov 30, 2025
b461ad8
Fix window closing if there's an invalid repo URL added.
Eauldane Nov 30, 2025
bfe338e
Formatting fix because of accidental backspace
Eauldane Nov 30, 2025
e518cf6
Remove error timeouts, have input validated on each change
Eauldane Dec 3, 2025
6cee8eb
Improve URI validation beyond just scheme validation
Eauldane Dec 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions Dalamud/Interface/Internal/Windows/Settings/SettingsWindow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,21 @@ public override void Draw()
{
if (buttonChild)
{
using var disabled = ImRaii.Disabled(this.tabs.Any(x => x.Entries.Any(y => !y.IsValid)));

using (ImRaii.PushStyle(ImGuiStyleVar.FrameRounding, 100f))
{
using var font = ImRaii.PushFont(InterfaceManager.IconFont);

var hasInvalidEntries = this.tabs.Any(x => x.Entries.Any(y => !y.IsValid));

using var disabled = ImRaii.Disabled(hasInvalidEntries);

if (ImGui.Button(FontAwesomeIcon.Save.ToIconString(), new Vector2(40)))
{
this.Save();

if (!ImGui.IsKeyDown(ImGuiKey.ModShift))
hasInvalidEntries = this.tabs.Any(x => x.Entries.Any(y => !y.IsValid));

if (!hasInvalidEntries && !ImGui.IsKeyDown(ImGuiKey.ModShift))
this.IsOpen = false;
}
}
Expand Down
78 changes: 55 additions & 23 deletions Dalamud/Interface/Internal/Windows/Settings/Widgets/ThirdRepoSettingsEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,21 @@ public override void Load()
this.thirdRepoList =
[.. Service<DalamudConfiguration>.Get().ThirdRepoList.Select(x => x.Clone())];
this.thirdRepoListChanged = false;
this.IsValid = true;
}

public override void Save()
{
var hasPendingRepo = !string.IsNullOrWhiteSpace(this.thirdRepoTempUrl);
var addedPendingRepo = this.TryAddTempRepo();
if (!addedPendingRepo && hasPendingRepo)
{
this.IsValid = false;
return;
}

this.IsValid = true;

Service<DalamudConfiguration>.Get().ThirdRepoList =
[.. this.thirdRepoList.Select(x => x.Clone())];

Expand Down Expand Up @@ -172,6 +183,8 @@ public override void Draw()
var url = thirdRepoSetting.Url;
if (ImGui.InputText($"##thirdRepoInput", ref url, 65535, ImGuiInputTextFlags.EnterReturnsTrue))
{
this.IsValid = true;

var contains = this.thirdRepoList.Select(repo => repo.Url).Contains(url);
if (thirdRepoSetting.Url == url)
{
Expand All @@ -189,8 +202,8 @@ public override void Draw()
}
else
{
this.thirdRepoListChanged = thirdRepoSetting.Url != url;
thirdRepoSetting.Url = url;
this.thirdRepoListChanged = url != thirdRepoSetting.Url;
}
}

Expand Down Expand Up @@ -229,33 +242,17 @@ public override void Draw()
ImGui.Text(repoNumber.ToString());
ImGui.NextColumn();
ImGui.SetNextItemWidth(-1);
ImGui.InputText("##thirdRepoUrlInput"u8, ref this.thirdRepoTempUrl, 300);
if (ImGui.InputText("##thirdRepoUrlInput"u8, ref this.thirdRepoTempUrl, 300))

@goaaats goaaats Dec 3, 2025

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.

@Eauldane Eauldane Dec 3, 2025

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!

{
this.IsValid = true;
}

ImGui.NextColumn();
// Enabled button
ImGui.NextColumn();
if (!string.IsNullOrEmpty(this.thirdRepoTempUrl) && ImGuiComponents.IconButton(FontAwesomeIcon.Plus))
{
this.thirdRepoTempUrl = this.thirdRepoTempUrl.TrimEnd();
if (this.thirdRepoList.Any(r => string.Equals(r.Url, this.thirdRepoTempUrl, StringComparison.InvariantCultureIgnoreCase)))
{
this.thirdRepoAddError = Loc.Localize("DalamudThirdRepoExists", "Repo already exists.");
Task.Delay(5000).ContinueWith(t => this.thirdRepoAddError = string.Empty);
}
else if (!ValidThirdPartyRepoUrl(this.thirdRepoTempUrl))
{
this.thirdRepoAddError = Loc.Localize("DalamudThirdRepoNotUrl", "The entered address is not a valid URL.\nDid you mean to enter it as a DevPlugin in the fields above instead?");
Task.Delay(5000).ContinueWith(t => this.thirdRepoAddError = string.Empty);
}
else
{
this.thirdRepoList.Add(new ThirdPartyRepoSettings
{
Url = this.thirdRepoTempUrl,
IsEnabled = true,
});
this.thirdRepoListChanged = true;
this.thirdRepoTempUrl = string.Empty;
}
this.TryAddTempRepo();
}

ImGui.Columns(1);
Expand All @@ -269,4 +266,39 @@ public override void Draw()
private static bool ValidThirdPartyRepoUrl(string url)
=> Uri.TryCreate(url, UriKind.Absolute, out var uriResult)
&& (uriResult.Scheme == Uri.UriSchemeHttps || uriResult.Scheme == Uri.UriSchemeHttp);

private bool TryAddTempRepo()
{
if (string.IsNullOrWhiteSpace(this.thirdRepoTempUrl))
return false;

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.");

@goaaats goaaats Dec 2, 2025

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

Task.Delay(5000).ContinueWith(t => this.thirdRepoAddError = string.Empty);
this.IsValid = false;
return false;
}

if (!ValidThirdPartyRepoUrl(this.thirdRepoTempUrl))
{
this.thirdRepoAddError = Loc.Localize("DalamudThirdRepoNotUrl", "The entered address is not a valid URL.\nDid you mean to enter it as a DevPlugin in the fields above instead?");
Task.Delay(5000).ContinueWith(t => this.thirdRepoAddError = string.Empty);
this.IsValid = false;
return false;
}

this.thirdRepoList.Add(new ThirdPartyRepoSettings
{
Url = this.thirdRepoTempUrl,
IsEnabled = true,
});
this.thirdRepoListChanged = true;
this.thirdRepoTempUrl = string.Empty;
this.IsValid = true;

return true;
}

}