Skip to content

Conversation

@guardrex
Copy link
Collaborator

Addresses #22444
Addresses #22045

@guardrex guardrex mentioned this pull request Jul 13, 2021
41 tasks
@guardrex
Copy link
Collaborator Author

guardrex commented Jul 13, 2021

Ahoy @TanayParikh! 🚢 ... I'm drafting the doc sections with examples for byte array interop:

  • This is only a draft for sure because I'm not on preview bits yet. I generally try to avoid the 🙉 Core Churn™ 🙉 until we get to RC1, then I go on the new bits and suspend my local work in released VS in favor of .NET CLI until the new VS releases at GA. Anyway, it just results in my needing to ask on a few points that I can't test and work out locally.

  • We have the two topics for interop: Call .NET from JS and Call JS from .NET. For these new sections+examples, I'd like to have:

    • The byte array go .NET to JS with a decoded string come back for the Call JS topic.
    • The byte array go JS to .NET with a decoded string (the same string ☝️ actually) come back for the Call .NET topic.

    In each case, the string is displayed to the dev, either in the component in the Call JS topic or via an alert() in the JS in the Call .NET topic.

    This layout of the coverage makes things clean and organized for the coverage with minimal reader confusion on the direction that the byte array is traveling (... and minimal doc author confusion, too! 😆).

Can you put an 👁️ on this PR at this early stage and see where the code is going to need a touch-up to make it work? If not, no worries. We can hold this PR in a draft state until I go on RC1 and then I'll work the 🐞 out of the code later.

BTW: Ignore the size-limits.md INCLUDES change here. That's just a NIT change that I want to make in passing. There's no INCLUDES action on this PR for the JS interop-byte array coverage ... each of the interop topics has a new section and everything is in each section.

@guardrex guardrex requested a review from TanayParikh July 13, 2021 15:53
Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @guardrex for putting this together. This looks great!

Left some general comments, still need to put it in a project and double check it compiles / works as expected. (I'll try to get to this, this week).

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 14, 2021

Thanks @TanayParikh ...

  • Yes, I do use BlazorSample. ByteArrayTesting was in here because that sample app is sitting out on my desktop waiting for me to adopt 6.0 locally. I fixed it. Eventually, this code will go into the snippet sample apps. Those are named BlazorSample.
  • Fixed the function/method naming to sendByteArray/SendByteArray.
  • We use arrow functions across the Blazor docs because the PU engineers usually send over their JS functions that way. I imagine that they like it because it's compact.
  • The <p> content at the bottom has to stay: The quote is from the movie, it's copyrighted, and CELA/Sony would have a fit if we didn't keep the attribution. This is common across the docs. I have all kinds of Star Wars, Star Trek, Dr. Who, etc. things in the docs for fun. 🚀👽 Every spot has the legal attribution with cross-links. We checked on this with CELA, and they were happy about the approach taken for these.

Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @guardrex! Verified the sample code is working as expected (with the following minor change to encoding). Could we go ahead with merging/publishing these docs? The feature was just announced and I'm sure users will find this super helpful! 😄

Also, should we update the menu bar as well? Otherwise, if users just run the app they'll have to manually navigate to the newly created pages. Not sure if there's a general convention for this situation in the docs.

        <div class="nav-item px-3">
            <NavLink class="nav-link" href="send-byte-array-js-to-dotnet">
                <span class="oi oi-arrow-right" aria-hidden="true"></span> JS to .NET Interop
            </NavLink>
        </div>
        <div class="nav-item px-3">
            <NavLink class="nav-link" href="send-byte-array-dotnet-to-js">
                <span class="oi oi-arrow-left" aria-hidden="true"></span> .NET to JS Interop
            </NavLink>
        </div>

@guardrex
Copy link
Collaborator Author

Thanks @TanayParikh ... We aren't showing navigation for every example later in the docs, such as at this point. It takes up too much space for little benefit. Early examples do show it (e.g., tutorials), and the Routing doc covers it in the Fundamentals node. We let those docs make devs aware of it.

I was going to update the component naming to match the snippet sample app later, but I went ahead and did that now. These examples will be added to the snippet sample apps sometime after GA. The docs don't build preview releases, at least not yet. We might build doc examples for RC.

No need to ping the others. They're all too busy to look at this 🏃😅. We worked out that the responsible engineer gets a single ping on these when the PR goes up. If they don't respond in a week or so, then Artak will look. He'll either be able to approve or reach out to the engineer for a look. In the end, I'm probably going to add a live doc coverage link to the topic section(s) for all of the 6.0 updates for a final management/engineer look before GA. For now tho, I only need your sign-off to merge, so I'll go ahead and get this in.

@guardrex guardrex marked this pull request as ready for review July 15, 2021 09:46
@guardrex guardrex merged commit ce727cc into main Jul 15, 2021
@guardrex guardrex deleted the guardrex/blazor-byte-array-jsinterop branch July 15, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants