-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gnoweb): rework & Implement new gnoweb design #3195
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
🛠 PR Checks Summary🔴 The pull request head branch must be up-to-date with its base (more info) Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Signed-off-by: gfanton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pr on fork incoming
bind string | ||
analytics bool | ||
json bool | ||
html bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why have this as an option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping the original behaviors of gnoweb
, it has also been added recently to gnodev by @moul, so I believe we still need to keep this around as an option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a README, not something too long / complex, but which can help me (or anyone else) quickly answer how to do changes and generally develop on gnoweb?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes definitely
assetsBase := "/" + strings.Trim(cfg.AssetsPath, "/") + "/" | ||
|
||
// Handle assets path | ||
mux.Handle(assetsBase, AssetHandler(true)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a way to configure this, maybe from gnoweb
or using a build tag, for local development
gno.land/pkg/markdown/toc.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after discussion, think this should be gno.land/pkg/gnoweb/markdown
we can move to be more generic in the future and then to gno.land/pkg/markdown, but i suggest you focus on the specific case for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comment: we never set a page title anywhere. Can we configure page titles? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I mean the <title>
head tag in HTML)
other general comment, but I'm sure you thought about it: it would be cool if the "search bar" (where the realm path is) acted kind of as a "command palette", and as such allowed to move quickly to help and source, and to move to other realms. I imagine this is somewhat envisioned/planned. I'm looking forward :) |
misc minor fixes and changes
Just another design point @alexiscolin |
Signed-off-by: gfanton <[email protected]>
"log/slog" | ||
"net/http" | ||
|
||
gnodev "github.com/gnolang/gno/contribs/gnodev/pkg/dev" | ||
"github.com/gnolang/gno/gno.land/pkg/gnoweb" | ||
gnoweb "github.com/gnolang/gno/gno.land/pkg/gnoweb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gnoweb "github.com/gnolang/gno/gno.land/pkg/gnoweb" | |
"github.com/gnolang/gno/gno.land/pkg/gnoweb" |
gno.land/cmd/gnoweb/README.md
Outdated
@@ -1,13 +0,0 @@ | |||
# gnoweb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep a README in this folder, but feel free to remove the "install" part and only mention that people can view previews on gno.land or run it with gnodev, linking to the gnodev folder.
@@ -1,14 +0,0 @@ | |||
package main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep it this way: 1. to facilitate the addition of second tests later, and 2. to ensure that go test
does not skip this folder and passes the basic syntax checks.
@@ -0,0 +1,12 @@ | |||
{{ define "breadcrumb" }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consider not committing those generated files (and gitignore them) and ensure that each install command runs go generate
?
go install -v . | ||
|
||
# Generate process | ||
generate: clean css ts fonts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should have a CI workflow to ensure it's always up-to-date
@alexiscolin another small bug; with ordered lists the number does not show up: |
Signed-off-by: gfanton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since faucet config is removed from config, would it not be accessible from Gnoweb anymore?
Which is the entrypoint of Faucet so far? Because CAPTCHA_SITE_KEY
is not set anywhere else, and is the one intended to be used by a Gnofaucet frontend.
cc: @zivkovicmilos
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
address #3191
Reworking the
gnoweb
package:gnoweb
new interface design(cc @alexiscolin).goldmark
for Markdown andchroma
for code highlighting, with almost no (in)direct dependencies.gnoweb
iteration.Preview
Home
Source
Docs
TODO: