Merge pull request #30058 from bdach/disabled-beatmap-comment-box

Properly disable comment box on things that cannot be commented on
This commit is contained in:
Dean Herbert 2024-10-01 18:52:40 +09:00 committed by GitHub
commit 598bc74614
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 99 additions and 28 deletions

View File

@ -12,6 +12,7 @@
using osu.Framework.Testing;
using osu.Game.Graphics.UserInterface;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Overlays;
using osu.Game.Overlays.Comments;
using osuTK;
@ -28,9 +29,10 @@ public partial class TestSceneCommentEditor : OsuManualInputManagerTestScene
private TestCancellableCommentEditor cancellableCommentEditor = null!;
private DummyAPIAccess dummyAPI => (DummyAPIAccess)API;
[SetUp]
public void SetUp() => Schedule(() =>
Add(new FillFlowContainer
[SetUpSteps]
public void SetUpSteps()
{
AddStep("create content", () => Child = new FillFlowContainer
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
@ -43,7 +45,8 @@ public void SetUp() => Schedule(() =>
commentEditor = new TestCommentEditor(),
cancellableCommentEditor = new TestCancellableCommentEditor()
}
}));
});
}
[Test]
public void TestCommitViaKeyboard()
@ -133,6 +136,34 @@ void assertLoggedOutState()
assertLoggedInState();
}
[Test]
public void TestCommentsDisabled()
{
AddStep("no reason for disable", () => commentEditor.CommentableMeta.Value = new CommentableMeta
{
CurrentUserAttributes = new CommentableMeta.CommentableCurrentUserAttributes(),
});
AddAssert("textbox enabled", () => commentEditor.ChildrenOfType<TextBox>().Single().ReadOnly, () => Is.False);
AddStep("specific reason for disable", () => commentEditor.CommentableMeta.Value = new CommentableMeta
{
CurrentUserAttributes = new CommentableMeta.CommentableCurrentUserAttributes
{
CanNewCommentReason = "This comment section is disabled. For reasons.",
}
});
AddAssert("textbox disabled", () => commentEditor.ChildrenOfType<TextBox>().Single().ReadOnly, () => Is.True);
AddStep("entire commentable meta missing", () => commentEditor.CommentableMeta.Value = null);
AddAssert("textbox enabled", () => commentEditor.ChildrenOfType<TextBox>().Single().ReadOnly, () => Is.False);
AddStep("current user attributes missing", () => commentEditor.CommentableMeta.Value = new CommentableMeta
{
CurrentUserAttributes = null,
});
AddAssert("textbox enabled", () => commentEditor.ChildrenOfType<TextBox>().Single().ReadOnly, () => Is.True);
}
[Test]
public void TestCancelAction()
{
@ -167,8 +198,7 @@ protected override void OnCommit(string value)
protected override LocalisableString GetButtonText(bool isLoggedIn) =>
isLoggedIn ? @"Commit" : "You're logged out!";
protected override LocalisableString GetPlaceholderText(bool isLoggedIn) =>
isLoggedIn ? @"This text box is empty" : "Still empty, but now you can't type in it.";
protected override LocalisableString GetPlaceholderText() => @"This text box is empty";
}
private partial class TestCancellableCommentEditor : CancellableCommentEditor
@ -189,7 +219,7 @@ protected override void OnCommit(string text)
}
protected override LocalisableString GetButtonText(bool isLoggedIn) => @"Save";
protected override LocalisableString GetPlaceholderText(bool isLoggedIn) => @"Multiline textboxes soon";
protected override LocalisableString GetPlaceholderText() => @"Multiline textboxes soon";
}
}
}

View File

@ -24,5 +24,14 @@ public class CommentableMeta
[JsonProperty("url")]
public string Url { get; set; } = string.Empty;
[JsonProperty("current_user_attributes")]
public CommentableCurrentUserAttributes? CurrentUserAttributes { get; set; }
public struct CommentableCurrentUserAttributes
{
[JsonProperty("can_new_comment_reason")]
public string? CanNewCommentReason { get; set; }
}
}
}

View File

@ -14,6 +14,8 @@
using osu.Game.Graphics.UserInterface;
using osu.Game.Graphics.UserInterfaceV2;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Resources.Localisation.Web;
using osuTK;
using osuTK.Graphics;
@ -21,6 +23,8 @@ namespace osu.Game.Overlays.Comments
{
public abstract partial class CommentEditor : CompositeDrawable
{
public Bindable<CommentableMeta?> CommentableMeta { get; set; } = new Bindable<CommentableMeta?>();
private const int side_padding = 8;
protected abstract LocalisableString FooterText { get; }
@ -53,8 +57,7 @@ public abstract partial class CommentEditor : CompositeDrawable
/// <summary>
/// Returns the placeholder text for the comment box.
/// </summary>
/// <param name="isLoggedIn">Whether the current user is logged in.</param>
protected abstract LocalisableString GetPlaceholderText(bool isLoggedIn);
protected abstract LocalisableString GetPlaceholderText();
protected bool ShowLoadingSpinner
{
@ -65,7 +68,7 @@ protected bool ShowLoadingSpinner
else
loadingSpinner.Hide();
updateCommitButtonState();
updateState();
}
}
@ -167,25 +170,33 @@ private void load(OverlayColourProvider colourProvider)
protected override void LoadComplete()
{
base.LoadComplete();
Current.BindValueChanged(_ => updateCommitButtonState(), true);
apiState.BindValueChanged(updateStateForLoggedIn, true);
Current.BindValueChanged(_ => updateState());
apiState.BindValueChanged(_ => Scheduler.AddOnce(updateState));
CommentableMeta.BindValueChanged(_ => Scheduler.AddOnce(updateState));
updateState();
}
protected abstract void OnCommit(string text);
private void updateCommitButtonState() =>
commitButton.Enabled.Value = loadingSpinner.State.Value == Visibility.Hidden && !string.IsNullOrEmpty(Current.Value);
private void updateStateForLoggedIn(ValueChangedEvent<APIState> state) => Schedule(() =>
private void updateState()
{
bool isAvailable = state.NewValue > APIState.Offline;
bool isOnline = apiState.Value > APIState.Offline;
LocalisableString? canNewCommentReason = CommentEditor.canNewCommentReason(CommentableMeta.Value);
bool commentsDisabled = canNewCommentReason != null;
bool canComment = isOnline && !commentsDisabled;
TextBox.PlaceholderText = GetPlaceholderText(isAvailable);
TextBox.ReadOnly = !isAvailable;
if (!isOnline)
TextBox.PlaceholderText = AuthorizationStrings.RequireLogin;
else if (canNewCommentReason != null)
TextBox.PlaceholderText = canNewCommentReason.Value;
else
TextBox.PlaceholderText = GetPlaceholderText();
TextBox.ReadOnly = !canComment;
if (isAvailable)
if (isOnline)
{
commitButton.Show();
commitButton.Enabled.Value = !commentsDisabled && loadingSpinner.State.Value == Visibility.Hidden && !string.IsNullOrEmpty(Current.Value);
logInButton.Hide();
}
else
@ -193,7 +204,25 @@ private void updateStateForLoggedIn(ValueChangedEvent<APIState> state) => Schedu
commitButton.Hide();
logInButton.Show();
}
});
}
// https://github.com/ppy/osu-web/blob/83816dbe24ad2927273cba968f2fcd2694a121a9/resources/js/components/comment-editor.tsx#L54-L60
// careful here, logic is VERY finicky.
private static LocalisableString? canNewCommentReason(CommentableMeta? meta)
{
if (meta == null)
return null;
if (meta.CurrentUserAttributes != null)
{
if (meta.CurrentUserAttributes.Value.CanNewCommentReason is string reason)
return reason;
return null;
}
return AuthorizationStrings.CommentStoreDisabled;
}
private partial class EditorTextBox : OsuTextBox
{

View File

@ -20,6 +20,7 @@
using JetBrains.Annotations;
using osu.Framework.Localisation;
using osu.Framework.Logging;
using osu.Game.Extensions;
using osu.Game.Graphics.Sprites;
using osu.Game.Resources.Localisation.Web;
using osu.Game.Users.Drawables;
@ -49,6 +50,7 @@ public partial class CommentsContainer : CompositeDrawable
private int currentPage;
private FillFlowContainer pinnedContent;
private NewCommentEditor newCommentEditor;
private FillFlowContainer content;
private DeletedCommentsCounter deletedCommentsCounter;
private CommentsShowMoreButton moreButton;
@ -114,7 +116,7 @@ private void load(OverlayColourProvider colourProvider)
Padding = new MarginPadding { Left = 60 },
RelativeSizeAxes = Axes.X,
AutoSizeAxes = Axes.Y,
Child = new NewCommentEditor
Child = newCommentEditor = new NewCommentEditor
{
OnPost = prependPostedComments
}
@ -242,6 +244,7 @@ protected void ClearComments()
protected void OnSuccess(CommentBundle response)
{
commentCounter.Current.Value = response.Total;
newCommentEditor.CommentableMeta.Value = response.CommentableMeta.SingleOrDefault(m => m.Id == id.Value && m.Type == type.Value.ToString().ToSnakeCase().ToLowerInvariant());
if (!response.Comments.Any())
{
@ -413,8 +416,7 @@ private partial class NewCommentEditor : CommentEditor
protected override LocalisableString GetButtonText(bool isLoggedIn) =>
isLoggedIn ? CommonStrings.ButtonsPost : CommentsStrings.GuestButtonNew;
protected override LocalisableString GetPlaceholderText(bool isLoggedIn) =>
isLoggedIn ? CommentsStrings.PlaceholderNew : AuthorizationStrings.RequireLogin;
protected override LocalisableString GetPlaceholderText() => CommentsStrings.PlaceholderNew;
protected override void OnCommit(string text)
{

View File

@ -428,7 +428,7 @@ private void toggleReply()
if (replyEditorContainer.Count == 0)
{
replyEditorContainer.Show();
replyEditorContainer.Add(new ReplyCommentEditor(Comment)
replyEditorContainer.Add(new ReplyCommentEditor(Comment, Meta)
{
OnPost = comments =>
{

View File

@ -2,6 +2,7 @@
// See the LICENCE file in the repository root for full licence text.
using System;
using System.Collections.Generic;
using System.Linq;
using osu.Framework.Allocation;
using osu.Framework.Localisation;
@ -26,12 +27,12 @@ public partial class ReplyCommentEditor : CancellableCommentEditor
protected override LocalisableString GetButtonText(bool isLoggedIn) =>
isLoggedIn ? CommonStrings.ButtonsReply : CommentsStrings.GuestButtonReply;
protected override LocalisableString GetPlaceholderText(bool isLoggedIn) =>
isLoggedIn ? CommentsStrings.PlaceholderReply : AuthorizationStrings.RequireLogin;
protected override LocalisableString GetPlaceholderText() => CommentsStrings.PlaceholderReply;
public ReplyCommentEditor(Comment parent)
public ReplyCommentEditor(Comment parent, IEnumerable<CommentableMeta> meta)
{
parentComment = parent;
CommentableMeta.Value = meta.SingleOrDefault(m => m.Id == parent.CommentableId && m.Type == parent.CommentableType);
}
protected override void LoadComplete()