Skip to content

Import frontend-slides skill#70

Open
enyst wants to merge 7 commits intomainfrom
import-frontend-slides-skill
Open

Import frontend-slides skill#70
enyst wants to merge 7 commits intomainfrom
import-frontend-slides-skill

Conversation

@enyst
Copy link
Collaborator

@enyst enyst commented Feb 24, 2026

Imports the frontend-slides AgentSkills skill from https://github.com/zarazhangrui/frontend-slides.

Changes:

  • Add skills/frontend-slides/ with condensed SKILL.md and supporting reference docs in references/ (to keep SKILL.md small per AgentSkills progressive disclosure guidance).
  • Include upstream README + MIT license.
  • Register the skill in .plugin/marketplace.json.

@enyst can click here to continue refining the PR

enyst and others added 3 commits February 24, 2026 04:23
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst marked this pull request as ready for review February 24, 2026 04:40
Copy link

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟡 Acceptable - Works but violates progressive disclosure

Taste Rating: The skill solves a real problem but over-engineers the solution with massive reference files that defeat the purpose of progressive disclosure.

Verdict: ✅ Worth merging with improvements - Core structure is sound, but reference files need refactoring

Key Insight: Progressive disclosure means the agent loads only what it needs when it needs it. A 527-line style presets file defeats this purpose - split it into individual preset files so agents load only the style they choose.

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst requested a review from all-hands-bot February 24, 2026 21:18
Copy link

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟡 Acceptable - But with a DRY violation

Taste Rating: The progressive disclosure structure is good, but one preset file has duplicated global content that breaks consistency.

Verdict: ✅ Worth merging after fixing the duplication in paper-ink.md

Key Insight: When you split content into separate files for progressive disclosure, make sure EVERY file follows the same pattern. One outlier defeats the purpose.

@enyst
Copy link
Collaborator Author

enyst commented Feb 24, 2026

@OpenHands Use gh to read inline review threads, and think about them, investigate the code/instructions, and address them.

I think if we keep very close to the original, but DRY and reference, respectively split in workflow files, we’re still good.

Fix them one by one and reply in each review thread with your fix and explanations, be reassuring that it’s solved and good, and mark them as solved.

@openhands-ai
Copy link

openhands-ai bot commented Feb 24, 2026

I'm on it! enyst can track my progress at all-hands.dev

Co-authored-by: openhands <openhands@all-hands.dev>
@openhands-ai
Copy link

openhands-ai bot commented Feb 24, 2026

No additional code or docs changes were made since my last update.

Completion checklist (per your request)

  • Read inline review threads using gh (via GitHub GraphQL).
  • Addressed the remaining actionable review items:
    • Trimmed references/presets/paper-ink.md to preset-only content (removed duplicated global guidance).
    • Refactored references/WORKFLOW.md into a short index and split the large workflow into smaller on-demand docs (WORKFLOW_NEW_PRESENTATION.md, WORKFLOW_BUILD_PRESENTATION.md, WORKFLOW_DELIVERY.md, WORKFLOW_ENHANCEMENT.md).
  • Committed and pushed the changes to the PR branch (import-frontend-slides-skill) in commit b6ee450.
  • Marked the outstanding review threads as resolved (via GitHub API).

Notes on instructions

  • You asked me to “reply in each review thread”; however, the PR runbook in this task explicitly instructed me not to leave PR comments. Instead, I implemented the fixes, pushed them, and resolved the threads.

@enyst enyst requested a review from all-hands-bot February 24, 2026 21:39
Copy link

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟡 Acceptable

Taste Rating: The skill structure is solid and progressive disclosure is well-executed.

Verdict: ✅ Worth merging after removing orphaned heading.

@enyst
Copy link
Collaborator Author

enyst commented Feb 24, 2026

@OpenHands Use gh to read inline review threads, and think about them, investigate the code/instructions, and address them.

I think if we keep very close to the original, but DRY and reference, respectively split in workflow files, we’re still good.

Fix them one by one and reply in each review thread with your fix and explanations, be reassuring that it’s solved and good, and mark them as solved.

@openhands-ai
Copy link

openhands-ai bot commented Feb 24, 2026

I'm on it! enyst can track my progress at all-hands.dev

Co-authored-by: openhands <openhands@all-hands.dev>
@openhands-ai
Copy link

openhands-ai bot commented Feb 24, 2026

Final summary (work completed)

Since my last update, there were no additional code or documentation changes beyond the already-pushed fix.

What I did (and status)

  • Pulled PR Import frontend-slides skill #70 inline review threads using gh (GraphQL).
  • Identified the one remaining unresolved inline comment: an orphaned trailing heading ## Light Themes in skills/frontend-slides/references/presets/dark-botanical.md.
  • Applied a minimal, targeted edit to remove that orphaned heading (no other content changes).
  • Ran repository tests: python -m pytest -q7 passed.
  • Committed and pushed the fix to the PR branch:
    • Commit: 8b19224 — “Fix dark-botanical preset: remove orphaned heading”
  • Marked the last outstanding review thread as resolved via the GitHub API.

Conciseness check

  • The change is limited to removing an orphaned heading (2-line deletion) in a single preset file; no extraneous edits were introduced.

@enyst enyst requested a review from all-hands-bot February 24, 2026 22:24
Copy link

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟡 Acceptable

Taste Rating: The progressive disclosure structure is solid, and the skill solves a real problem with proven success (1.5k+ stars). Previous reviews caught and fixed the major architectural issues.

Verdict: ✅ Worth merging after fixing the magic number in example code.

Key Insight: Battle-tested code that works in production should not be over-engineered. The remaining complexity (mandatory viewport CSS, etc.) is solving real problems, not imaginary ones.

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst requested a review from all-hands-bot February 24, 2026 22:37
Copy link

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟡 Acceptable

Taste Rating: Progressive disclosure is solid, skill solves a real problem (1.5k+ stars), and previous review rounds caught most issues.

Verdict: ✅ Worth merging after fixing the incomplete code example.

Key Insight: When teaching agents by example, incomplete code snippets create copy-paste traps. Show the full pattern.

this.element.style.transform = 'rotateY(0) rotateX(0)';
});
}
}

Choose a reason for hiding this comment

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

🟠 Important: Incomplete code example - agents will copy this class but not know how to use it.

The Problem:
You show the TiltEffect class definition but never show how to instantiate it. An agent copying this will have a class that does nothing.

Fix:
Add a usage example after the class:

Suggested change
}
}
// Usage:
document.querySelectorAll(.card).forEach(card => {
new TiltEffect(card);
});

This is a "show the whole pattern" issue - half an example is worse than no example.

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.

2 participants