Skip to content

Layout adjustments - #2095

Draft
osma wants to merge 11 commits into
mainfrom
feat-layout-tweaks
Draft

osma wants to merge 11 commits into
mainfrom
feat-layout-tweaks

Conversation

@osma

@osma osma commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Reasons for creating this PR

There are some rough spots in the current Skosmos layout. This PR proposes some fixes.

Link to relevant issue(s), if any

Description of the changes in this PR

  1. Make the alphabetical layout tighter
  2. Adjust border between properties so it only spans the property value column
  3. Reduce main content margins slightly
  4. Switch rdf:type icon, fix tooltips and reduce empty space on the search result page
  5. Fix some more alignment issues on the search results page
  6. Change the rdf:type icon again, to a diamond symbol

Known problems or uncertainties in this PR

Keyboard navigation support for tooltips is not implemented in this PR. We have issue #1967 open regarding that.

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma added this to the 3.5 milestone Oct 1, 2026
@osma osma self-assigned this Oct 1, 2026
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.89%. Comparing base (2c4ead9) to head (1d26a14).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2095   +/-   ##
=========================================
  Coverage     70.89%   70.89%           
  Complexity     1718     1718           
=========================================
  Files            34       34           
  Lines          4463     4463           
=========================================
  Hits           3164     3164           
  Misses         1299     1299           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@osma
osma force-pushed the feat-layout-tweaks branch from 330f7fd to 0e938f1 Compare October 1, 2026 13:33
@osma
osma force-pushed the feat-layout-tweaks branch from 0e938f1 to 20a8b40 Compare October 1, 2026 13:35
@osma
osma requested a balanced review from Copilot October 1, 2026 13:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Duplicate description IDs, keyboard-inaccessible tooltips, layout regressions, and stale tests remain unresolved.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity

Open (5)
What changed in this PR

Adjusts page spacing and search-result layouts while adding explanatory icon tooltips.

Changes:

  • Refines spacing across search, vocabulary, and concept views.
  • Adds accessible descriptions and hover tooltips to result icons.
  • Aligns search-result icons and values using flexbox.
File Description
src/​view/​vocab-search.twig Adjusts result-heading spacing.
src/​view/​vocab-info.inc.twig Reduces section padding.
src/​view/​search-results.inc.twig Adds icon tooltips, descriptions, and layout wrappers.
src/​view/​global-search.twig Adjusts result-heading spacing.
src/​view/​concept-card.inc.twig Refines spacing and property-value styling.
resource/​css/​skosmos.css Updates spacing, borders, tooltips, and result alignment.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/view/search-results.inc.twig
Comment thread resource/css/skosmos.css
Comment thread resource/css/skosmos.css Outdated
Comment thread src/view/search-results.inc.twig Outdated
Comment thread src/view/search-results.inc.twig
@osma osma changed the title WIP: Layout adjustments Layout adjustments Oct 1, 2026
@osma
osma requested a balanced review from Copilot October 1, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Truncated rows clip the new tooltips, and the regression test does not exercise a comma-containing description.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Tooltip is clipped inside hidden search result rows

src/​view/​search-results.inc.twig:37

These t-top tooltips are rendered inside list items that truncateSearchResults() changes to .search-result-hidden. That class has overflow: hidden (resource/css/skosmos.css:1631-1636), so on every truncated row the tooltip is translated above the item and clipped, making the newly added hover explanation invisible. The clipping needs to be applied to the value span rather than the whole row (or the tooltip must render outside the clipped container).

Low severity Preserve spacing after notation icon

src/​view/​search-results.inc.twig:15

The following {%~ if concept.notation %} strips the whitespace after this icon, so removing the explicit   renders the arrow directly adjacent to the notation or concept link. Add an explicit margin to preserve separation regardless of Twig whitespace trimming.

Comment on lines +75 to +76
it('Show-all count ignores tooltip descriptions that contain commas', () => {
cy.visit(`/yso/en/search?clang=en&q=euro`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Under review

Development

Successfully merging this pull request may close these issues.

2 participants