Convert the frontend project to ASP.NET #112
Conversation
There was a problem hiding this comment.
Thanks for starting to work on this! I've added a couple comments. One thing I'd like to do is... drumroll... avoid any js library (and that includes javascript). We can make this work on Chrome and Firefox only, so we can do things like async and use fetch.
Make sure to nuke the secrets you've shared here.
| @@ -0,0 +1,61 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk.Web"> | |||
There was a problem hiding this comment.
can we use the new csproj format? so that we don't list each and every file ;)
There was a problem hiding this comment.
This is just the project as Visual Studio created it, of course we can tune it as appropriate. I'll need to investigate first what that new format it about, though. 😅
Konamiman
force-pushed
the
convert-frontend-project-to-asp-net
branch
from
0f01410
to
7259178
Compare
- Create the project named Asp.Net.Web, targetting ASP.Net Core 2.2 - Add Core and Migrations project references - Add controllers and models from Traducir.Api as linked files - Add the same NuGet packages that Traducir.Api has - Run setup configuration from Traducir.Api (via linked Setup.cs file)
- Only login/logout and string counts work for now - The port for thr Traducir.Web.Net project is now 5000, therfore this project can be used instead of Traducir.Api as the backend for the old Traducir.Web project.
Konamiman
force-pushed
the
convert-frontend-project-to-asp-net
branch
from
7259178
to
d9a854c
Compare
… new StringsService class in the Api project
|
Since this is a total diferent back and front, wouldn't be better to have this in a new project? |
|
@topcatarg Keeping everything in the same solution actually eases the conversion project, as the old and the new frontends can be compared and run side by side. Also the new backend project is actually using the same code files as the old one. |
|
@topcatarg I agree with @Konamiman, my end goal is to completely kill the Api and Rect projects from the solution. Keeping everything on the same project eases PR reviews greatly (as otherwise, contributors would need to have PRs for both backend and frontend, and automated tools would need to know which frontend PR matches which backend PR) |
Traducir.Web.Net/wwwroot/css/miniprofiler-includes.4.0.138.gcc91adf599.css
Outdated
Show resolved
Hide resolved
- Filters and initial string list are properly initialized on first page load based on the supplied query string - Modifying the filters loads the new data via Ajax and pushes the newly generated query string in browser history, same for the quick filter links - Reviewers see "Ignore" buttons in the list, but the buttons do nothing yet - A spinner is displayed in the center of the screen while loading Ajax data Ohter improvements: - Html.DropDownListFor is now used for rendering the dropdown filters - Bundling+minification of css and js configured for production environment - ASP.NET client validation disabled on startup (no more validation attributes in the generated HTML)
Konamiman
force-pushed
the
convert-frontend-project-to-asp-net
branch
from
d9aa7fb
to
68e5212
Compare
…only. for now it just allows to login as different user types.
- Fixes in dev-only "login as" mechanism - Added ignored and push status filters in UI, for >=TrustedUser only - Rendered results are now limited to 200 even if no predicate is present (which will happen if the selected ignore filter is "all")
…k yet) and: - Added the app/impersonate route - Added entries to login as three fake users (that are automatically created in the db) in the development menu - Bootstrap Native is now used instead of the regular Bootstrap. - jQuery removed from the project.
Note: for suggestion creation error messages the standard browser alert popup is used instead of the Bootstrap modal, because Bootstrap Native doesn't support opening a modal on top of another one, and thus it closes the suggestions editor if an alert modal is shown.
| } | ||
|
|
||
| for (const event of events) { | ||
| document.addEventListener(event, e => { |
There was a problem hiding this comment.
@Konamiman I was puzzled when debugging this... from what I see, we're setting all the events on the document and then when the event is triggered, we check if the target is in the selector.
I guess the reasoning here is to avoid having to hook elements dynamically created with the events... but I'm a bit worried about this introducing unexpected effects... also, every time there's a click on the document (even if it's not in an element) we check each selector.
I can't help but feel we'd be better of with using events attached to the actual dom elements (it would also make it easier to debug it).
I'm not saying we need to tweak this right now, just that it's something I'd like to address (even post-merge).
What do you think?
There was a problem hiding this comment.
Yes, we check each selector on each click, but since this is a very small project I don't think there's a huge performance penalty or noticeable side effect, so I prioritized code simplicity. If/when we discover that this approach has indeed problems (or if the project grows), we can think about changing it.
…r fails with "The script resource is behind a redirect, which is disallowed")
A temptative and WIP implementation of #106 Kill the react frontend.
The new Visual Studio project is temporarily named
Traducir.Web.Netwhile development is in progress. Cleanup to do after the conversion is finished:Copy all the relevant controller and model files from
Traducir.Api, replacing the linked files; adjust namespaces.Copy the code from
Traducir.Api.Setup.csinto the equivalent file in the new project, removing the linkedTraducir.Api/Setup.csfile.Delete the
Traducir.ApiandTraducir.Webprojects.Rename the project from
Traducir.Web.NettoTraducir.Web.