Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… url in program.sc to resolve api calls
created deploy script to deal with fingerprinted file errors and automate deployment process updated gitignore file
Removed unused files, implemented a new NavBar using Nimble Anchor tags, changed the styling of the app, and solved the routing issue (see README).
Cleaned up branch to only include the Blazor Wasm App changes
examples/Web Apps/Framework Examples/Blazor/BlazorWasmAuthExample/Layout/NavBar.razor.css
Show resolved
Hide resolved
examples/Web Apps/Framework Examples/Blazor/BlazorWasmAuthExample/README.html
Outdated
Show resolved
Hide resolved
jattasNI
left a comment
There was a problem hiding this comment.
Some feedback to address. But good news: I was able to build and run the app locally and it looks and behaves just like the React one. Nice!
| @@ -1,4 +1,4 @@ | |||
| # SystemLink Enterprise Examples | |||
| # SystemLink Enterprise Examples - BYU team Branch | |||
There was a problem hiding this comment.
This change should be reverted before going in to the NI repo.
| #auto | ||
| ../../../../DotNet |
There was a problem hiding this comment.
This section looks strange: for a .gitignore at the root I wouldn't expect relative paths reaching up out of the root directory.
| @@ -0,0 +1,431 @@ | |||
| # Blazor WebAssembly API Authentication Example | |||
There was a problem hiding this comment.
Could you take a pass at aligning the naming and READMEs across React and Blazor? Things I noticed:
- directory names are different (
ApiKeyAuthAppvsBlazorWasmAuthExample). I think my proposal would be to make the directory names the same, as long as we can make sure the built nipkgs get different names that include the framework. I don't have a strong preference about what the name should be. - Align the contents of the README to have the same description of what the app does but unique descriptions of the framework-specific setup.
- Align the level of detail. I think this README has too much: it should just describe the prerequisites and step-by-step build instructions, doesn't need to explain the architecture or implementation details. (But the workaround for fingerprinting is really good information, that should stay)
| @@ -0,0 +1,17 @@ | |||
| # Build output | |||
There was a problem hiding this comment.
I think you could remove some of this since a lot of these paths are listed in the root .gitignore. Any files that are generic to .NET (like bin/obj/*.suo) can go in the root. Any content that's specific to this app (like README_files) should stay here.
| @using System.Text.Json | ||
|
|
||
| <div class="button-and-title"> | ||
| <nimble-button class="button" appearance-variant="accent" @onclick="HandleClick"> |
There was a problem hiding this comment.
Similar to my comment about the theme provider, this should use the Blazor component NimbleButton rather than the custom element nimble-button.
| working-directory: ${{ env.APP_DIR }} | ||
| run: dotnet publish *.csproj -o dist | ||
|
|
||
| - name: Copy fingerprinted dotnet.js |
There was a problem hiding this comment.
Instead of writing custom code in the pipeline script to apply this workaround, could we do one of these instead?
- Update the Blazor project to invoke the
deploy.shscript as part of every build - Update the pipeline to invoke the deploy.sh script
1 is preferable because it applies the fix in a way that users could take advantage of it in their own web app development, even if they don't use github actions. But 2 is acceptable if you can't figure out the MSBuild commands needed to invoke a script during build.
To do 1 I think you'd need to split the deploy script into separate scripts for applying the fingerprinting workaround and for using the slcli to upload the app. That seems like a good direction too: again, it would help users leverage that workaround even if they have their own flow for building and uploading the nipkg.
| └── .gitignore ← Ignores appsettings.Development.json | ||
| ``` | ||
|
|
||
| ## How It Works |
There was a problem hiding this comment.
Could you refactor this to be more clear getting started steps where each step includes the action required. Right now there's a lot of explanatory content that is making it hard to tell exactly what the minimum steps are to launch the app.
|
|
||
| ## Prerequisites | ||
|
|
||
| - .NET 10.0 SDK or higher |
There was a problem hiding this comment.
It would be good to link to where you go to download this.
|
|
||
| ## Related Examples | ||
|
|
||
| - [Blazor Server Example](../BlazorApp1/) — Server-side rendering with SignalR, |
There was a problem hiding this comment.
This ended up not being part of this PR so we can remove it from the README.
| @@ -0,0 +1,42 @@ | |||
| <div class="header"> | |||
| <a href="https://github.com/Samuelsotogit/systemlink-enterprise-examples-fork" | |||
There was a problem hiding this comment.
This link needs to be updated to point at the NI repo.
Main points of this pull request:
Suggested testing steps: