fix(pages): add Pages deploy workflow, Turnstile bot protection, and CSS/SEO fixes - #22
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new contact submission flow should reset Turnstile on failed submissions, and the updated headline CSS selectors should be scoped as intended to avoid style bleed/conflicts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR prepares the Jekyll site for production launch by adding a GitHub Pages deploy workflow, integrating Cloudflare Turnstile protection + an API-backed contact submission flow, and cleaning up SEO/CSS issues (including removing tracked build artifacts).
Changes:
- Added GitHub Actions workflow to build and deploy the Jekyll site to GitHub Pages on pushes to
main. - Added Cloudflare Turnstile widget and updated contact form JS to POST submissions to the configured contact API endpoint.
- Removed tracked
_site/artifacts and tightened repo/Jekyll hygiene via.gitignoreand_config.ymlexcludes; updatedGemfile.lockplatforms/checksums for Linux CI.
File summaries
| File | Description |
|---|---|
| .github/workflows/deploy.yml | Adds automated Jekyll build + GitHub Pages deployment workflow. |
| .gitignore | Ignores Jekyll build outputs/caches and vendor directory. |
| _config.yml | Updates site URL and excludes build/dependency artifacts from the Jekyll build. |
| Gemfile.lock | Adds Linux platforms/checksums and updates bundled Bundler version metadata. |
| assets/css/terms.css | Removes inline headline styling dependency and adjusts headline selector usage for terms page. |
| assets/css/vlei.css | Removes inline headline styling dependency and adjusts headline selector usage for vLEI page. |
| assets/js/contact.js | Adds Turnstile token enforcement and API submission via fetch(). |
| contact.html | Adds Turnstile widget + script and improves heading hierarchy (h2 → h1). |
| terms-and-conditions.html | Removes inline hero headline styles in favor of CSS. |
| vlei-credentials.html | Removes inline hero headline styles in favor of CSS. |
| _site/index.html | Removes previously tracked generated homepage artifact. |
| _site/contact/index.html | Removes previously tracked generated contact page artifact. |
| _site/terms-and-conditions/index.html | Removes previously tracked generated terms page artifact. |
| _site/vlei-credentials/index.html | Removes previously tracked generated vLEI page artifact. |
| _site/assets/js/main.js | Removes previously tracked generated JS artifact. |
| _site/assets/js/home.js | Removes previously tracked generated JS artifact. |
| _site/assets/js/contact.js | Removes previously tracked generated JS artifact. |
| _site/assets/css/home.css | Removes previously tracked generated CSS artifact. |
| _site/assets/css/contact.css | Removes previously tracked generated CSS artifact. |
| _site/assets/css/terms.css | Removes previously tracked generated CSS artifact. |
| _site/assets/css/vlei.css | Removes previously tracked generated CSS artifact. |
| _site/assets/css/press-room.css | Removes previously tracked generated CSS artifact. |
| _site/assets/img/logo-light.svg | Removes previously tracked generated image asset. |
| _site/assets/img/logo-dark.svg | Removes previously tracked generated image asset. |
| _site/assets/img/favicon.svg | Removes previously tracked generated image asset. |
Review details
- Files reviewed: 21/72 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
The Pages workflow should explicitly upload the built _site directory, and the new contact JS introduces modern syntax (optional chaining/async) that may not be compatible with the repo’s otherwise ES5-style browser scripts.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/deploy.yml:33
actions/upload-pages-artifactis invoked without specifying the artifact path. The action examples typically setwith: path: _site(or your custom destination). Being explicit avoids deploying an empty or wrong directory if defaults change.
assets/js/contact.js:9- The submit handler is declared
asyncand usesawait, which introduces a syntax-level compatibility requirement (no transpile step is evident, and other site JS is ES5-style). Consider rewriting this block using Promise chaining (fetch(...).then(...).catch(...).finally(...)) to avoid parse errors in older/embedded browsers.
This issue also appears on line 24 of the same file.
assets/js/contact.js:24
- Optional chaining (
?.) is the only usage in the non-vendored site JS and will cause a hard syntax error in browsers that don’t support it. Since these scripts appear to ship without transpilation, use a null check instead.
- Files reviewed: 21/72 changed files
- Comments generated: 0 new
- Review effort level: Lite
GitHub Actions
Inactive
There was a problem hiding this comment.
🟡 Changes recommended
The workflow can deploy from a non-main branch and the updated contact script introduces non-ES5 syntax inconsistent with the repo’s current frontend baseline.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
assets/js/contact.js:9
- The submit handler is declared as
asyncand usesawait, which deviates from the ES5-style convention used by other first-party scripts in assets/js (e.g., assets/js/main.js and assets/js/home.js). Without a transpile step, this can cause runtime syntax errors in older browsers.
assets/js/contact.js:24 - Optional chaining (
?.) is used here, but the repo’s in-house scripts in assets/js follow an ES5-style baseline; this syntax will break in environments without optional chaining support unless you add a transpile step.
- Files reviewed: 21/72 changed files
- Comments generated: 1
- Review effort level: Lite
…build artifact path
There was a problem hiding this comment.
🔵 Needs a closer look
The deploy workflow configuration appears able to deploy from a non-main branch and uses action version tags that may not exist, risking failed or unintended production deployments.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
assets/js/contact.js:9
- The submit handler is now
asyncand usesawait, whereas other first-party scripts (assets/js/main.js, assets/js/home.js) avoid async/await. If the site is intended to remain “no-transpile” / broadly compatible, consider rewriting this handler using Promise chaining instead of async/await.
assets/js/contact.js:24 - This uses optional chaining (
?.), which is inconsistent with the project’s ES5-style first-party scripts (e.g., assets/js/main.js, assets/js/home.js) and can break on older browsers without transpilation.
assets/js/contact.js:65 turnstile.reset()relies on an implicit global variable; usingwindow.turnstile.reset()avoids a potential ReferenceError in environments whereturnstileisn’t bound as a global identifier (even ifwindow.turnstileexists).
.github/workflows/deploy.yml:5
- The deploy workflow is configured to run on pushes to both
mainandwebsite-revamp-fixes. Since the PR description says production publishing should happen when changes are merged tomain, this extra branch can unintentionally deploy from a non-main branch and overwrite the Pages site.
branches: ["main", "website-revamp-fixes"]
- Files reviewed: 21/72 changed files
- Comments generated: 0 new
- Review effort level: Lite
…nd add architecture docs
GitHub Actions
Inactive
There was a problem hiding this comment.
🟡 Changes recommended
The Pages workflow deploy trigger includes a non-main branch and the new contact script introduces non-ES5 syntax (async/await and optional chaining) inconsistent with the repo’s existing browser-safe JS style.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
assets/js/contact.js:24
- Optional chaining (
?.) is used here to read the Turnstile token. Likeasync/await, this can cause a hard parse error in older browsers when served without transpilation. Use a simple null check instead so the script stays ES5-compatible.
.github/workflows/deploy.yml:6 - The workflow is configured to deploy on pushes to
website-revamp-fixesas well asmain. The PR description says automated publishing should happen when changes are merged tomain; deploying from a non-default branch can publish unreviewed/temporary changes to production.
push:
branches: ["main", "website-revamp-fixes"]
workflow_dispatch:
- Files reviewed: 22/73 changed files
- Comments generated: 1
- Review effort level: Lite
GitHub Actions
Inactive
There was a problem hiding this comment.
🟡 Changes recommended
The contact form script introduces modern JavaScript syntax inconsistent with existing ES5-style assets, and the deploy workflow trigger does not match the documented/claimed deployment behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
assets/js/contact.js:9
- The submit handler is now marked
asyncand usesawaitlater in the function. Other site scripts underassets/jsare written in ES5-style syntax; without a transpile step this can reduce browser compatibility and diverge from established conventions.
.github/workflows/deploy.yml:5 - The PR/README describe deploying on push to
main(and optionally release branches), but this workflow also deploys fromwebsite-revamp-fixes. If production deploys should only happen frommain, this extra branch should be removed (or the docs/PR description should be updated accordingly).
branches: ["main", "website-revamp-fixes"]
- Files reviewed: 22/73 changed files
- Comments generated: 1
- Review effort level: Lite
…it or optional chaining
GitHub Actions
Inactive
There was a problem hiding this comment.
🟡 Changes recommended
The updated contact form JavaScript introduces trailing commas in object literals (risking hard parse errors in older browsers) and the README’s deploy trigger description is currently inconsistent with the workflow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
README.md:102
- README says deploys trigger on
mainor an “active release branch”, but the workflow currently triggers onmainandwebsite-revamp-fixes. This mismatch can confuse operators trying to understand when production deploys happen.
assets/js/contact.js:59
- The fetch options object also uses a trailing comma after the last property (
body). If the goal is maximum ES5-era browser compatibility, this can likewise cause a hard parse error in older engines.
- Files reviewed: 22/73 changed files
- Comments generated: 1
- Review effort level: Lite
GitHub Actions
Inactive
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/maintainability documentation issues (README trigger mismatch, new trailing commas in assets/js/contact.js, and leftover unscoped !important headline rules in CSS) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
assets/css/vlei.css:658
- Although the headline styles are now scoped under
.vlei-wrap, there is still a global.page-headline em { ... !important; }rule later in this file (around line 961). That unscoped!importantselector can leak styling to any future.page-headlineusage outside the vLEI page and undermines the intent of scoping.
assets/css/terms.css:658 - Although the headline styles are now scoped under
.terms-wrap, there is still a global.page-headline em { ... !important; }rule later in this file (around line 945). That unscoped!importantselector can leak styling to any future.page-headlineusage outside the Terms page and undermines the intent of scoping.
assets/js/contact.js:53
- The payload object literal ends with a trailing comma after the last property. Since this repo is intentionally keeping scripts broadly compatible (ES5-style, no transpile), it’s safer to avoid introducing trailing commas in object literals.
assets/js/contact.js:59 - The
fetchoptions object includes a trailing comma after the last property (body). For maximum browser compatibility (and consistency with other scripts inassets/js), avoid trailing commas in object literals.
- Files reviewed: 22/73 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The changes are cohesive and low-risk overall, with only a small README/workflow trigger mismatch noted for follow-up.
Review details
Suppressed comments (1)
README.md:104
- README claims the Pages deploy workflow triggers on push to
main"or active release branch", but.github/workflows/deploy.ymlis currently only configured for pushes tomain(plusworkflow_dispatch). This makes the CI/CD docs inaccurate; either update the README to match the workflow, or expand the workflow triggers to include release branches.
Deployment is fully automated through GitHub Actions (`.github/workflows/deploy.yml`):
- **Trigger**: Every push to the default branch (`main`) or active release branch.
- **Build**: Compiles Jekyll assets with `bundle exec jekyll build --destination ./_site`.
- **Deploy**: Packages and uploads the `_site/` directory to GitHub Pages using `actions/deploy-pages`.
- Files reviewed: 22/73 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
There are at least two correctness/compatibility risks in the updated build/client code (lockfile platform mismatch and a JS trailing-comma syntax hazard) that should be addressed before merging.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
assets/js/contact.js:59
- The
fetchoptions object literal has a trailing comma after the last property (body), which can be a hard syntax error in older JS engines. Since the repo otherwise avoids newer syntax for compatibility, it’s safer to remove the trailing comma here.
- Files reviewed: 22/73 changed files
- Comments generated: 0 new
- Review effort level: Lite
Why These Changes Are Needed
This update prepares the new Jekyll site for a smooth production launch:
main, removing the need for any manual build or upload steps.Technical Summary
.github/workflows/deploy.ymlconfiguring automated Jekyll build and GitHub Pages deployment on push tomain.x86_64-linuxandx86_64-linux-gnuplatforms with native gem checksums toGemfile.lockto ensure CI runner compatibility.contact.html) and client submission logic (assets/js/contact.js).https://inbound-inquiries.provenant.net/contactwith runtime override viawindow.CONTACT_API_URL.<h2>to<h1>with matched visual styling (56px) for proper heading hierarchy.vlei-credentials.htmlandterms-and-conditions.html, resolving descendant selector conflicts inassets/css/vlei.cssandassets/css/terms.css._site/build artifacts from version control and aligns_config.ymlurlwith apexCNAME(https://provenant.net).