From 56c41a614a5cb5ddcbbc6235c622d162d81d0220 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Wed, 26 Feb 2020 17:38:50 +0300 Subject: [PATCH] Remove CommentsPage component --- .../Online/TestSceneCommentsContainer.cs | 3 +- .../Visual/Online/TestSceneCommentsPage.cs | 232 ------------------ .../Overlays/Comments/CommentsContainer.cs | 134 ++++++++-- osu.Game/Overlays/Comments/CommentsPage.cs | 161 ------------ 4 files changed, 112 insertions(+), 418 deletions(-) delete mode 100644 osu.Game.Tests/Visual/Online/TestSceneCommentsPage.cs delete mode 100644 osu.Game/Overlays/Comments/CommentsPage.cs diff --git a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs index ece280659c..b9938ab25b 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs @@ -27,8 +27,7 @@ namespace osu.Game.Tests.Visual.Online typeof(OverlaySortTabControl<>), typeof(ShowChildrenButton), typeof(DeletedCommentsCounter), - typeof(VotePill), - typeof(CommentsPage), + typeof(VotePill) }; protected override bool UseOnlineAPI => true; diff --git a/osu.Game.Tests/Visual/Online/TestSceneCommentsPage.cs b/osu.Game.Tests/Visual/Online/TestSceneCommentsPage.cs deleted file mode 100644 index a28a0107a1..0000000000 --- a/osu.Game.Tests/Visual/Online/TestSceneCommentsPage.cs +++ /dev/null @@ -1,232 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System; -using System.Collections.Generic; -using osu.Game.Overlays.Comments; -using osu.Game.Overlays; -using osu.Framework.Allocation; -using osu.Game.Online.API.Requests.Responses; -using osu.Game.Users; -using osu.Game.Graphics.UserInterface; -using osu.Framework.Bindables; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics; -using osuTK; -using JetBrains.Annotations; -using NUnit.Framework; - -namespace osu.Game.Tests.Visual.Online -{ - public class TestSceneCommentsPage : OsuTestScene - { - public override IReadOnlyList RequiredTypes => new[] - { - typeof(DrawableComment), - typeof(CommentsPage), - }; - - [Cached] - private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Purple); - - private readonly BindableBool showDeleted = new BindableBool(); - private readonly Container content; - - private TestCommentsPage commentsPage; - - public TestSceneCommentsPage() - { - Add(new FillFlowContainer - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical, - Spacing = new Vector2(0, 10), - Children = new Drawable[] - { - new Container - { - AutoSizeAxes = Axes.Y, - Width = 200, - Child = new OsuCheckbox - { - Current = showDeleted, - LabelText = @"Show Deleted" - } - }, - content = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - } - } - }); - } - - [Test] - public void TestAppendDuplicatedComment() - { - AddStep("Create page", () => createPage(getCommentBundle())); - AddAssert("Dictionary length is 10", () => commentsPage?.DictionaryLength == 10); - AddStep("Append existing comment", () => commentsPage?.AppendComments(getCommentSubBundle())); - AddAssert("Dictionary length is 10", () => commentsPage?.DictionaryLength == 10); - } - - [Test] - public void TestEmptyBundle() - { - AddStep("Create page", () => createPage(getEmptyCommentBundle())); - AddAssert("Dictionary length is 0", () => commentsPage?.DictionaryLength == 0); - } - - private void createPage(CommentBundle commentBundle) - { - commentsPage = null; - content.Clear(); - content.Add(commentsPage = new TestCommentsPage(commentBundle) - { - ShowDeleted = { BindTarget = showDeleted } - }); - } - - private CommentBundle getEmptyCommentBundle() => new CommentBundle - { - Comments = new List(), - }; - - private CommentBundle getCommentBundle() => new CommentBundle - { - Comments = new List - { - new Comment - { - Id = 1, - Message = "Simple test comment", - LegacyName = "TestUser1", - CreatedAt = DateTimeOffset.Now, - VotesCount = 5 - }, - new Comment - { - Id = 100, - Message = "This comment has \"load replies\" button because it has unloaded replies", - LegacyName = "TestUser1100", - CreatedAt = DateTimeOffset.Now, - VotesCount = 5, - RepliesCount = 2, - }, - new Comment - { - Id = 111, - Message = "This comment has \"Show More\" button because it has unloaded replies, but some of them are loaded", - LegacyName = "TestUser1111", - CreatedAt = DateTimeOffset.Now, - VotesCount = 100, - RepliesCount = 2, - }, - new Comment - { - Id = 112, - ParentId = 111, - Message = "I'm here to make my parent work", - LegacyName = "someone", - CreatedAt = DateTimeOffset.Now, - VotesCount = 2, - }, - new Comment - { - Id = 2, - Message = "This comment has been deleted :( but visible for admins", - LegacyName = "TestUser2", - CreatedAt = DateTimeOffset.Now, - DeletedAt = DateTimeOffset.Now, - VotesCount = 5 - }, - new Comment - { - Id = 3, - Message = "This comment is a top level", - LegacyName = "TestUser3", - CreatedAt = DateTimeOffset.Now, - RepliesCount = 2, - }, - new Comment - { - Id = 4, - ParentId = 3, - Message = "And this is a reply", - RepliesCount = 1, - LegacyName = "TestUser1", - CreatedAt = DateTimeOffset.Now, - }, - new Comment - { - Id = 15, - ParentId = 4, - Message = "Reply to reply", - LegacyName = "TestUser1", - CreatedAt = DateTimeOffset.Now, - }, - new Comment - { - Id = 6, - ParentId = 3, - LegacyName = "TestUser11515", - CreatedAt = DateTimeOffset.Now, - DeletedAt = DateTimeOffset.Now, - }, - new Comment - { - Id = 5, - Message = "This comment is voted and edited", - LegacyName = "BigBrainUser", - CreatedAt = DateTimeOffset.Now, - EditedAt = DateTimeOffset.Now, - VotesCount = 1000, - EditedById = 1, - } - }, - IncludedComments = new List(), - UserVotes = new List - { - 5 - }, - Users = new List - { - new User - { - Id = 1, - Username = "Good_Admin" - } - }, - }; - - private CommentBundle getCommentSubBundle() => new CommentBundle - { - Comments = new List - { - new Comment - { - Id = 1, - Message = "Simple test comment", - LegacyName = "TestUser1", - CreatedAt = DateTimeOffset.Now, - VotesCount = 5 - }, - }, - IncludedComments = new List(), - }; - - private class TestCommentsPage : CommentsPage - { - public TestCommentsPage(CommentBundle commentBundle) - : base(commentBundle) - { - } - - public new void AppendComments([NotNull] CommentBundle bundle) => base.AppendComments(bundle); - - public int DictionaryLength => CommentDictionary.Count; - } - } -} diff --git a/osu.Game/Overlays/Comments/CommentsContainer.cs b/osu.Game/Overlays/Comments/CommentsContainer.cs index 591a9dc86e..ef833b9345 100644 --- a/osu.Game/Overlays/Comments/CommentsContainer.cs +++ b/osu.Game/Overlays/Comments/CommentsContainer.cs @@ -9,10 +9,12 @@ using osu.Framework.Graphics; using osu.Framework.Bindables; using osu.Framework.Graphics.Shapes; using osu.Game.Online.API.Requests.Responses; -using System.Threading; using System.Linq; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Users; +using System.Collections.Generic; +using JetBrains.Annotations; +using osu.Game.Graphics.Sprites; namespace osu.Game.Overlays.Comments { @@ -30,7 +32,6 @@ namespace osu.Game.Overlays.Comments private IAPIProvider api { get; set; } private GetCommentsRequest request; - private CancellationTokenSource loadCancellation; private int currentPage; private FillFlowContainer content; @@ -151,9 +152,8 @@ namespace osu.Game.Overlays.Comments return; request?.Cancel(); - loadCancellation?.Cancel(); request = new GetCommentsRequest(id.Value, type.Value, Sort.Value, currentPage++, 0); - request.Success += onSuccess; + request.Success += response => Schedule(() => onSuccess(response)); api.PerformAsync(request); } @@ -166,44 +166,132 @@ namespace osu.Game.Overlays.Comments content.Clear(); } + private readonly Dictionary commentDictionary = new Dictionary(); + private void onSuccess(CommentBundle response) { - loadCancellation = new CancellationTokenSource(); - - LoadComponentAsync(new CommentsPage(response) + if (!response.Comments.Any()) { - ShowDeleted = { BindTarget = ShowDeleted }, - Sort = { BindTarget = Sort }, - Type = { BindTarget = type }, - CommentableId = { BindTarget = id } - }, loaded => + content.Add(new NoCommentsPlaceholder()); + return; + } + else { - content.Add(loaded); + appendComments(response); deletedCommentsCounter.Count.Value += response.Comments.Count(c => c.IsDeleted && c.IsTopLevel); + } - if (response.HasMore) + if (response.HasMore) + { + int loadedTopLevelComments = 0; + content.Children.OfType().ForEach(p => loadedTopLevelComments++); + + moreButton.Current.Value = response.TopLevelCount - loadedTopLevelComments; + moreButton.IsLoading = false; + } + else + { + moreButton.Hide(); + } + } + + /// + /// Appends retrieved comments to the subtree rooted of comments in this page. + /// + /// The bundle of comments to add. + private void appendComments([NotNull] CommentBundle bundle) + { + var orphaned = new List(); + + foreach (var comment in bundle.Comments.Concat(bundle.IncludedComments)) + { + // Exclude possible duplicated comments. + if (commentDictionary.ContainsKey(comment.Id)) + continue; + + addNewComment(comment); + } + + // Comments whose parents were seen later than themselves can now be added. + foreach (var o in orphaned) + addNewComment(o); + + void addNewComment(Comment comment) + { + var drawableComment = getDrawableComment(comment); + + if (comment.ParentId == null) { - int loadedTopLevelComments = 0; - content.Children.OfType().ForEach(p => loadedTopLevelComments += p.Children.OfType().Count()); - - moreButton.Current.Value = response.TopLevelCount - loadedTopLevelComments; - moreButton.IsLoading = false; + // Comments that have no parent are added as top-level comments to the flow. + content.Add(drawableComment); + } + else if (commentDictionary.TryGetValue(comment.ParentId.Value, out var parentDrawable)) + { + // The comment's parent has already been seen, so the parent<-> child links can be added. + comment.ParentComment = parentDrawable.Comment; + parentDrawable.Replies.Add(drawableComment); } else { - moreButton.Hide(); + // The comment's parent has not been seen yet, so keep it orphaned for the time being. This can occur if the comments arrive out of order. + // Since this comment has now been seen, any further children can be added to it without being orphaned themselves. + orphaned.Add(comment); } + } + } - commentCounter.Current.Value = response.Total; - }, loadCancellation.Token); + private DrawableComment getDrawableComment(Comment comment) + { + if (commentDictionary.TryGetValue(comment.Id, out var existing)) + return existing; + + return commentDictionary[comment.Id] = new DrawableComment(comment) + { + ShowDeleted = { BindTarget = ShowDeleted }, + Sort = { BindTarget = Sort }, + RepliesRequested = onCommentRepliesRequested + }; + } + + private void onCommentRepliesRequested(DrawableComment drawableComment, int page) + { + var request = new GetCommentsRequest(id.Value, type.Value, Sort.Value, page, drawableComment.Comment.Id); + + request.Success += response => Schedule(() => appendComments(response)); + + api.PerformAsync(request); } protected override void Dispose(bool isDisposing) { request?.Cancel(); - loadCancellation?.Cancel(); base.Dispose(isDisposing); } + + private class NoCommentsPlaceholder : CompositeDrawable + { + [BackgroundDependencyLoader] + private void load(OverlayColourProvider colourProvider) + { + Height = 80; + RelativeSizeAxes = Axes.X; + AddRangeInternal(new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = colourProvider.Background4 + }, + new OsuSpriteText + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + Margin = new MarginPadding { Left = 50 }, + Text = @"No comments yet." + } + }); + } + } } } diff --git a/osu.Game/Overlays/Comments/CommentsPage.cs b/osu.Game/Overlays/Comments/CommentsPage.cs deleted file mode 100644 index 9b146b0a7d..0000000000 --- a/osu.Game/Overlays/Comments/CommentsPage.cs +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Allocation; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics; -using osu.Framework.Bindables; -using osu.Game.Online.API.Requests.Responses; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics.Sprites; -using System.Linq; -using osu.Game.Online.API.Requests; -using osu.Game.Online.API; -using System.Collections.Generic; -using JetBrains.Annotations; - -namespace osu.Game.Overlays.Comments -{ - public class CommentsPage : CompositeDrawable - { - public readonly BindableBool ShowDeleted = new BindableBool(); - public readonly Bindable Sort = new Bindable(); - public readonly Bindable Type = new Bindable(); - public readonly BindableLong CommentableId = new BindableLong(); - - [Resolved] - private IAPIProvider api { get; set; } - - private readonly CommentBundle commentBundle; - private FillFlowContainer flow; - - public CommentsPage(CommentBundle commentBundle) - { - this.commentBundle = commentBundle; - } - - [BackgroundDependencyLoader] - private void load(OverlayColourProvider colourProvider) - { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - - AddRangeInternal(new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = colourProvider.Background5 - }, - flow = new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - } - }); - - if (!commentBundle.Comments.Any()) - { - flow.Add(new NoCommentsPlaceholder()); - return; - } - - AppendComments(commentBundle); - } - - private DrawableComment getDrawableComment(Comment comment) - { - if (CommentDictionary.TryGetValue(comment.Id, out var existing)) - return existing; - - return CommentDictionary[comment.Id] = new DrawableComment(comment) - { - ShowDeleted = { BindTarget = ShowDeleted }, - Sort = { BindTarget = Sort }, - RepliesRequested = onCommentRepliesRequested - }; - } - - private void onCommentRepliesRequested(DrawableComment drawableComment, int page) - { - var request = new GetCommentsRequest(CommentableId.Value, Type.Value, Sort.Value, page, drawableComment.Comment.Id); - - request.Success += response => Schedule(() => AppendComments(response)); - - api.PerformAsync(request); - } - - protected readonly Dictionary CommentDictionary = new Dictionary(); - - /// - /// Appends retrieved comments to the subtree rooted of comments in this page. - /// - /// The bundle of comments to add. - protected void AppendComments([NotNull] CommentBundle bundle) - { - var orphaned = new List(); - - foreach (var comment in bundle.Comments.Concat(bundle.IncludedComments)) - { - // Exclude possible duplicated comments. - if (CommentDictionary.ContainsKey(comment.Id)) - continue; - - addNewComment(comment); - } - - // Comments whose parents were seen later than themselves can now be added. - foreach (var o in orphaned) - addNewComment(o); - - void addNewComment(Comment comment) - { - var drawableComment = getDrawableComment(comment); - - if (comment.ParentId == null) - { - // Comments that have no parent are added as top-level comments to the flow. - flow.Add(drawableComment); - } - else if (CommentDictionary.TryGetValue(comment.ParentId.Value, out var parentDrawable)) - { - // The comment's parent has already been seen, so the parent<-> child links can be added. - comment.ParentComment = parentDrawable.Comment; - parentDrawable.Replies.Add(drawableComment); - } - else - { - // The comment's parent has not been seen yet, so keep it orphaned for the time being. This can occur if the comments arrive out of order. - // Since this comment has now been seen, any further children can be added to it without being orphaned themselves. - orphaned.Add(comment); - } - } - } - - private class NoCommentsPlaceholder : CompositeDrawable - { - [BackgroundDependencyLoader] - private void load(OverlayColourProvider colourProvider) - { - Height = 80; - RelativeSizeAxes = Axes.X; - AddRangeInternal(new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = colourProvider.Background4 - }, - new OsuSpriteText - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - Margin = new MarginPadding { Left = 50 }, - Text = @"No comments yet." - } - }); - } - } - } -}