Blueprints: Update Blueprint deeplink step design#2397
Conversation
|
Thanks for the review, Wojtek.
Do you mean to parse the contents of the Blueprint JSON and use it to generate a message like the above? I think that could work nicely until we have a dedicated way to output and let the user manage the contents of the Blueprint we discussed earlier.
This is something that feels redundant to me - as all Blueprints there are loaded from a deeplink URL (whether the JSON is included in the deeplink or referred from a different source). I have asked Marina earlier today about it in Figma: RToz6tIuQ7nlZrikBte4GU-fi-10352_72525#1587850255. I am happy to add the footer there. If we implement the "Blueprint installs X plugins, Y themes, and runs Z blocks of PHP code" part for Blueprints that don't have any title/description, it may not feel that redundant.
Good idea. I have created a separate task to look into that: https://linear.app/a8c/issue/STU-1209/improve-the-display-of-blueprint-deeplink-errors |
| { blueprintDescription } | ||
| <VStack className="max-w-[400px] min-w-[250px] max-h-[172px] mx-auto p-6 border rounded-lg border-gray-200 mt-16"> | ||
| <HStack className="h-full" alignment="top" spacing={ 4 }> | ||
| <Icon className="fill-a8c-blue-50 shrink-0" icon={ check } size={ 29 } /> |
There was a problem hiding this comment.
I think it is a good idea to keep the consistency here with what we have in Studio even if it might be a bit different from design 👍
There was a problem hiding this comment.
I did not mind that there was not a lot of information about the blueprint but to me, the spacing was slightly off when it was a shorter version:
I am wondering if maybe moving the title closer to the icon would bring a better visual balance for this case.
Alternatively, it wouldbe solved if it showed more content.
Yes, but we can add that as a follow-up.
As this step will also allow the selection of a file from the user's computer (as in the import step), the footer will differentiate between the two cases: a file loaded from a URL and a file selected from the local system. Also, I'm unsure if we need "Blueprint selected" text - shouldn't this text be the Blueprint title? Then we could have:
Possible variations:
|
28d2fd5 to
24f60a9
Compare
📊 Performance Test ResultsComparing f35b54f vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
|
Thank you for your review and suggestions, Kat & Wojtek! I have made some of the changes and will continue next week. I have set the PR back to draft and will let you know when it will be ready for review agian. |
22d06ef to
2bc8be5
Compare
|
I have updated the UI and also included the
For this case where the description isn't available I have created a dedicated task for this change: STU-1224. |
wojtekn
left a comment
There was a problem hiding this comment.
Looks good.
Do we need a "Blueprint selected" header for anything? Or could we replace this header content with blueprintTitle value?
Also, could we update jsbin with content that allows us to easily test all possible cases?
| <Icon icon={ check } size={ 24 } className="text-green-600" /> | ||
| <Text className="text-xl font-medium text-gray-900">{ __( 'Blueprint selected' ) }</Text> | ||
| <Text className="text-base font-medium text-gray-900 max-w-md px-4" weight={ 400 }> | ||
| { blueprintTitle } |
There was a problem hiding this comment.
I'm unsure if this came up before, but should we filter/sanitize that data?
There was a problem hiding this comment.
I would say that in this case a clamp on the length should be enough. I have added it in 2b244d3.
The Blueprint JSON also gets validated before we get title and description out of it.
There was a problem hiding this comment.
Yep, React escapes any tags included in those, but we could consider some sanitization, too.
I like it there - in relation to the checkmark icon on the side. It is based on the Figma design. I am also open to replacing it with Blueprint title. @Marinatsu what do you think? Here's how it can look like:
Yes, we can update that to have more possibilities for testing in one place. |
|
@ivan-ottinger I think it's much better without that static title. It's cleaner, closer to the import UI, and thanks to "Start from Blueprint" screen header it's still clear where user lands:
|
Sounds good. 👍🏼 I have removed the extra text in e22f508. |
I have added more testing deeplinks in https://output.jsbin.com/kulaqijega to test this PR more easily. We can extend that later for the PR where we add parsing for JSON contents and could also consider a different place than jsbin for those test deeplinks. 🙂 |
2b244d3 to
f35b54f
Compare
|
All cases look great. Thanks for updating jsbin - let's use this one for subsequent PRs that target this feature. |







Related issues
Proposed Changes
ℹ️ Please note there are several differences compared to the design proposed in Figma (RToz6tIuQ7nlZrikBte4GU-fi-10353_7411), for example the check icon is blue in this PR to match existing design of checkmarks we already have in Studio.
Testing Instructions
npm install && npm start.Pre-merge Checklist