Closed Leo6Leo closed 1 week ago
Built without sensitive environment variables
Name | Link |
---|---|
Latest commit | 10b0ae6338621edb02859f77ccf41cc939e81139 |
Latest deploy log | https://app.netlify.com/sites/knative/deploys/66748d7a707cdc0008522b90 |
Deploy Preview | https://deploy-preview-6007--knative.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Thanks Leo, this is great!
I think we should play with the link titles here.
"Bookstore tutorial" and "Quickstart", wdyt?
I think we should play with the link titles here.
"Bookstore tutorial" and "Quickstart", wdyt?
I can create a tread and ask how UX WG think, and whether designers have some ideas on it! The thread for this discussion is here.
@Leo6Leo instead of adding another tab to the top level navigation (which is already too full according to the UX group), can we put the bookstore tutorial under the already existing Tutorial tab? Maybe we can have a start page there that opens to either the quickstart tutorial or the e2e bookstore tutorial?
@Leo6Leo instead of adding another tab to the top level navigation (which is already too full according to the UX group), can we put the bookstore tutorial under the already existing Tutorial tab? Maybe we can have a start page there that opens to either the quickstart tutorial or the e2e bookstore tutorial?
Yeah I do agree, the current top level navigation is really packed. And a start page idea sounds good to me! I will reflect the changes in this PR.
How's about adding it in sidebar of getting-started-page ?
And adding a button in home page which can will take users directly to tutorial page, just like it is there in NextJs home page
Header | Header |
---|---|
Yes, I agree with @Cali0707. Current nav bar already been tightly packed :) Hopefully we have new design getting it narrow down. Yes, it's good to have in quick start tutorial that current explore knative button in the home page redirects there
Something from the user perspective, a user(this can be me) can be curious to see what's newly added to the Knative. For an instance, we have introduced this Bookstore tutorial or new releases or new events etc.. It can make a quick access and overview of the what's the latest news of Knative. Can we add any section of What's new? If yes, we can add this beside Needs to Know more, something it seems like we are using more space here :)
cc: @knative/ux-wg-leads
Hey @asr2003 that's a neat idea, but I don't think this is the best spot to discuss that. Normally on PRs we just discuss the changes involved in this PR, not ideas for other changes. I would recommend opening an issue in the UX repository or starting a new thread on slack
Just got this idea while exploring this PR, so i have dropped my idea here without further missing any from my mind :)
Okay, we can continue this in our existing thread in ux channel
/hold Will unhold on June 11 when it is ready to be merged
I have found some necessary UI improvements on the Doc Feature and listed all in a new issue req-https://github.com/knative/docs/issues/6012
I think the URLs are a little cryptic, can we call pages the same as the title of the page with dash for spaces ?
the mode button is huge compared to other components
the mode button is huge compared to other components
Should we make it smaller? This is the design from one of the designers in the UX WG.
@matzew is this PR something you can take a look at?
A nice, and lengthly tutorial
This is something that we could use for like 3 hiur workshops at conferences.
Very nice !
LGTM
I am not sure if some of @pierDipi's comments need to be addressed as well - mine are cosmetics, and I added hints what I found sub-optimal, feel free to polish this in a different PR
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Leo6Leo, pierDipi
The full list of commands accepted by this bot can be found here.
The pull request process is described here
/unhold
/cherry-pick release-1.14
@matzew: #6007 failed to apply on top of branch "release-1.14":
Applying: feat: add the first three pages of the sample app tutorial
Applying: Apply suggestions from code review
Applying: fix: fix the review comments in pg0 and pg1
Applying: feat: add blog post 2
Applying: feat: adding the images for page 2
Applying: add: add more pages
Applying: add: adding the rest of the pages
Applying: add: modify the nav
Applying: add: finishing polishing
Applying: fix: update the iframe embedded video link
Applying: fix: fix the video embedding dimension and the access
Applying: fix: link minor error fix
Applying: fix: hide the setup script section
Applying: Apply suggestions from code review
Applying: Update docs/bookstore/page-0/page-0-intro.md
Applying: Update docs/bookstore/page-0.5/pg0.5-env-setup.md
Applying: fix: fix christoph's comment
Applying: fix: Remove all the extra new lines
Applying: fix: update kubernetes to Kubernetes
Applying: Apply suggestions from code review
Applying: fix: fix the wrong indentation in the yaml files
.git/rebase-apply/patch:107: trailing whitespace.
.git/rebase-apply/patch:139: trailing whitespace.
```yaml
.git/rebase-apply/patch:183: trailing whitespace.
badwordfilter:
.git/rebase-apply/patch:205: trailing whitespace.
```yaml
.git/rebase-apply/patch:279: trailing whitespace.
```yaml
warning: 5 lines add whitespace errors.
Using index info to reconstruct a base tree...
A code-samples/eventing/bookstore-sample-app/solution/node-server/config/100-event-display.yaml
A code-samples/eventing/bookstore-sample-app/start/slack-sink/application.properties
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): code-samples/eventing/bookstore-sample-app/start/slack-sink/application.properties deleted in HEAD and modified in fix: fix the wrong indentation in the yaml files. Version fix: fix the wrong indentation in the yaml files of code-samples/eventing/bookstore-sample-app/start/slack-sink/application.properties left in tree.
CONFLICT (modify/delete): code-samples/eventing/bookstore-sample-app/solution/node-server/config/100-event-display.yaml deleted in HEAD and modified in fix: fix the wrong indentation in the yaml files. Version fix: fix the wrong indentation in the yaml files of code-samples/eventing/bookstore-sample-app/solution/node-server/config/100-event-display.yaml left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0021 fix: fix the wrong indentation in the yaml files
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
ok, than not :-)
fixes #5938
Proposed Changes