Created UiMessageAlert for Blazorise implementation #5712
Conversation
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Volo.Abp.AspNetCore.Authentication.OpenIdConnect", "src\Volo.Abp.AspNetCore.Authentication.OpenIdConnect\Volo.Abp.AspNetCore.Authentication.OpenIdConnect.csproj", "{DEFE3DB2-EA4F-4F90-87FC-B25D64427BC5}" | ||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Volo.Abp.AspNetCore.Authentication.OpenIdConnect", "src\Volo.Abp.AspNetCore.Authentication.OpenIdConnect\Volo.Abp.AspNetCore.Authentication.OpenIdConnect.csproj", "{DEFE3DB2-EA4F-4F90-87FC-B25D64427BC5}" |
There was a problem hiding this comment.
I don't know why all these project GUID changes. Does it hurt the solution? :)
There was a problem hiding this comment.
I have no idea why this happens. it's also happening on my own projects sometimes. Never had any problems. I can revert it if needed?
| Task NotifyConfirmationReceivedAsync(string message, string title, TaskCompletionSource<bool> callback); | ||
| } | ||
|
|
||
| public class UiMessageEventArgs : EventArgs |
There was a problem hiding this comment.
Every type (class, interface, enum...) should be localized in its own file.
| [Dependency(ReplaceServices = true)] | ||
| public class BlazoriseUiMessageService : IUiMessageService, IScopedDependency | ||
| { | ||
| private readonly IUiMessageNotifierService uiMessageNotifierService; |
There was a problem hiding this comment.
This class almost does nothing but delegates the work to the IUiMessageNotifierService. Also, IUiMessageNotifierService has no other purpose. Should we remove IUiMessageNotifierService (and its implementations) and merge its logic to this class?
There was a problem hiding this comment.
I removed IUiMessageNotifierService and just used UiMessageNotifierService
|
|
||
| sb.Append($"color:{MessageIconColor}"); | ||
|
|
||
| return sb.ToString(); |
There was a problem hiding this comment.
I suppose you will enhance this method. Otherwise, simply returning $"color:{MessageIconColor}" would be better.
There was a problem hiding this comment.
The idea was to add more features in the future so I would just leave it like this.
| @@ -15,6 +15,7 @@ | |||
| <ItemGroup> | |||
| <ProjectReference Include="..\..\..\..\..\framework\src\Volo.Abp.Autofac.WebAssembly\Volo.Abp.Autofac.WebAssembly.csproj" /> | |||
| <ProjectReference Include="..\..\..\..\..\framework\src\Volo.Abp.AspNetCore.Components.WebAssembly.BasicTheme\Volo.Abp.AspNetCore.Components.WebAssembly.BasicTheme.csproj" /> | |||
| <ProjectReference Include="..\..\..\..\..\framework\src\Volo.Abp.BlazoriseUI\Volo.Abp.BlazoriseUI.csproj" /> | |||
There was a problem hiding this comment.
We should not need to reference the Volo.Abp.BlazoriseUI.csproj from here. Because Volo.Abp.AspNetCore.Components.WebAssembly.BasicTheme (indirectly) depends on the Volo.Abp.BlazoriseUI.csproj project.
BTW, if you would need to add an ABP package, you always should add a [DependsOn(...)] attribute to the module class in your project (see the doc).
| @@ -14,16 +14,41 @@ | |||
| <h5 class="m-1"> <i class="fas fa-rocket"></i> Congratulations, <strong>MyProjectName</strong> is successfully running!</h5> | |||
| </Badge> | |||
|
|
|||
| <Card> | |||
There was a problem hiding this comment.
We should not commit such test code to GitHub. This is the startup template that is used when a developer wants to create a new solution, so we don't wan't to make it dirty :)
There was a problem hiding this comment.
I know this is just for testing. I was expecting for you to remove it after you review the UI :)
| using Microsoft.AspNetCore.Components; | ||
| using Volo.Abp.AspNetCore.Components.WebAssembly; | ||
|
|
||
| namespace MyCompanyName.MyProjectName.Blazor.Pages |
There was a problem hiding this comment.
Same comment is valid here (see my comment for the Index.razor)
It's still WIP but has some basic functionality: