Skip to content

Fix layout issues and align with Discourse core CSS conventions - #23

Open
jrgong420 wants to merge 8 commits into
naidihr:masterfrom
jrgong420:fix/comprehensive-improvements
Open

Fix layout issues and align with Discourse core CSS conventions#23
jrgong420 wants to merge 8 commits into
naidihr:masterfrom
jrgong420:fix/comprehensive-improvements

Conversation

@jrgong420

Copy link
Copy Markdown

Summary

This PR fixes several layout issues and aligns the theme component with Discourse core CSS variable naming conventions and patterns.

Changes Made

1. Discourse Core CSS Variable Alignment

  • ✅ Adopted Discourse core CSS variables:
    • --category-header-logo-size (maps from size_logo setting)
    • --category-description-width (760px, Discourse default)
    • --category-header-border (uses --content-border-color)
  • ✅ Replaced hardcoded spacing with Discourse tokens:
    • --space-2, --space-4, --space-6
  • ✅ Implemented core's aspect-image pattern for responsive logo sizing
  • ✅ Removed non-standard --category-bg-image variable

2. Layout Fixes

  • Fixed desktop layout: Description now appears below title (not beside it)
  • Fixed stacking issue: When "Inline read more" is disabled, elements no longer stack vertically
    • Introduced .category-title-text-wrapper to group title, description, and read-more link
    • Simplified grid from 2×2 to 2×1 with text wrapper handling vertical stacking
  • Moved category-about-url inside text wrapper for consistent layout

3. Mobile Improvements

  • ✅ Changed force_mobile_alignment default to false (maintains left-right layout on mobile by default)
  • ✅ Added responsive mobile logo sizing with clamp(48px, 18vw, 96px)
  • ✅ Updated mobile stacked mode to work with new wrapper structure

Layout Structure

Before:

.category-title-contents (flex with wrap)
  ├── .category-logo
  ├── .category-title-name
  └── .category-title-description

.category-about-url (outside, causes stacking)

After:

.category-title-contents (grid: 2 columns)
  ├── .category-logo (column 1)
  └── .category-title-text-wrapper (column 2, flex column)
      ├── .category-title-name
      ├── .category-title-description
      └── .category-about-url (when not inline)

Visual Result

Desktop (logo left):

┌─────────────────────────────────────┐
│ [Logo]  Category Title              │
│         Category Description...     │
│         Read more link              │
└─────────────────────────────────────┘

Benefits

  1. Standards Compliance: Uses Discourse core CSS variables and patterns
  2. Robust Layout: Grid structure is stable regardless of settings
  3. Better Maintainability: Cleaner CSS with semantic grouping
  4. Mobile-Friendly: Responsive logo sizing and optional stacked mode
  5. Future-Proof: Aligned with Discourse theming best practices

Files Changed

  • common/common.scss - Core styling updates
  • mobile/mobile.scss - Mobile responsive behavior
  • settings.yml - Default setting change for force_mobile_alignment
  • javascripts/discourse/components/category-header.gjs - Template structure with wrapper

Testing

Please test:

  • ✅ Desktop layout with logo left/right/top
  • ✅ Mobile layout (both default and stacked mode)
  • ✅ "Inline read more" enabled/disabled
  • ✅ Different logo sizes (standard/small/original)
  • ✅ Categories with/without logos
  • ✅ Long descriptions and titles

Pull Request opened by Augment Code with guidance from the PR author

Critical Fixes:
- Fix image/description overlap by replacing float layout with flexbox
- Fix mobile selectors to target actual component classes
- Fix header visibility logic for hide_if_no_category_description setting

High Priority:
- Add accessibility features (ARIA, keyboard support) to toggle link
- Implement lazy loading with caching for full category descriptions

Medium Priority:
- Refactor inline styles to CSS custom properties
- Improve mobile responsive design with proper flexbox

Low Priority:
- Replace hard-coded colors with Discourse theme variables
- Use router service for SPA-friendly route checks
- Update about.json with version metadata

Performance improvements:
- Lazy load full descriptions only when needed
- Cache descriptions by category ID to prevent redundant requests
- Reduce inline style strings by 80%

Accessibility improvements:
- Add role=button, aria-expanded, aria-controls to toggle
- Implement Enter/Space keyboard activation
- Add unique IDs for ARIA references

See CHANGELOG.md for detailed information.
- Adopt Discourse core CSS variables (--category-header-logo-size, --category-description-width, --category-header-border)
- Replace hardcoded spacing with Discourse tokens (--space-2, --space-4, --space-6)
- Implement core's aspect-image pattern for responsive logo sizing
- Fix desktop layout: description now appears below title (not beside it)
- Fix stacking issue when 'Inline read more' is disabled by introducing .category-title-text-wrapper
- Move category-about-url inside text wrapper for consistent layout
- Change force_mobile_alignment default to false (maintains left-right layout on mobile)
- Remove non-standard --category-bg-image variable
- Add responsive mobile logo sizing with clamp(48px, 18vw, 96px)
- Simplify grid layout from 2x2 to 2x1 with text wrapper handling vertical stacking
- Add title_text_size setting with options: smallest, smaller, normal, larger, largest
- Default to 'normal' to preserve Discourse core default behavior
- Apply font size control to .category-title-name h1 using Discourse font tokens
- Bump theme version from 2.0.0 to 2.1.0
- Mirrors existing description_text_size pattern for consistency
Remove all legacy SelectKit fallback code and use only the modern
DMenu-based category-notifications-dropdown component (Discourse 3.5+).

Changes:
- Remove SelectKit fallback logic from categoryNotificationsComponentName
- Remove legacy arguments (modalForMobile, asModalOnMobile) from wrapper
- Clean up debug logging to remove SelectKit-specific checks
- Remove SelectKit-specific mobile overflow CSS rules
- Bump minimum_discourse_version to 3.5.0
- Bump theme_version to 2.2.0

Benefits:
- Fixes mobile modal bug (was showing desktop dropdown)
- Cleaner, more maintainable code (-40 lines)
- Aligned with Discourse 3.5+ standards
- DMenu automatically handles mobile (modal) vs desktop (popover)

Desktop: Bell opens DMenu popover
Mobile: Bell opens DMenu modal with .d-modal.fk-d-menu-modal classes
- Replace legacy SelectKit prop API with modern DMenu pattern
- Update category-notifications-wrapper to use @levelId/@onchange props
- Add currentUser guard to prevent errors for anonymous users
- Remove dynamic component detection and RequireJS fallback code
- Simplify bell rendering in category-header template

The bell icon now updates immediately when users change notification
levels, using Discourse core's reactive Category.setNotification() flow.
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.

1 participant