Skip to content
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

Closes #1822 - Move page title block to content_top region #1834

Closed
wants to merge 3 commits into from

Conversation

akslay
Copy link
Contributor

@akslay akslay commented Aug 10, 2022

Description

Moves the page title block to the content_top region to improve page title clarity and prevent the title from displaying below the sidebar_first region on mobile.

Review site: https://7b5e2985-cedf-4a13-ab91-3a35916c835d--pr-1834.probo.build/

Related issues

Closes #1822

Related to: #1835

Types of changes

Arizona Quickstart (install profile, custom modules, custom theme)

  • Patch release changes
    • Bug fix
    • Accessibility, performance, or security improvement
    • Critical institutional link or brand change
  • Minor release changes
    • New feature
    • Breaking or visual change to existing behavior
    • Non-critical brand change
    • New internal API or API improvement with backwards compatibility
    • Risky or disruptive cleanup to comply with coding standards
    • High-risk or disruptive change (requires upgrade path, risks regression, etc.)
  • Other or unknown

Drupal core

  • Patch release changes
    • Security update
    • Patch level release (non-security bug-fix release)
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major or minor level update
  • Other or unknown

Drupal contrib projects

  • Patch release changes
    • Security update
    • Patch or minor level update
    • Add new module
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major level update
  • Other or unknown

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@akslay akslay added the high priority Must get done for this milestone label Aug 10, 2022
@akslay akslay self-assigned this Aug 10, 2022
@akslay akslay requested a review from a team as a code owner August 10, 2022 21:47
@akslay akslay added the visual change Introduces a visual change label Aug 10, 2022
@danahertzberg
Copy link
Contributor

This will update existing sites. Please wait to merge until we run it by leadership.

@trackleft
Copy link
Member

trackleft commented Aug 17, 2022

Patch existing sites like this:

"extra": {
    "patches": {
       "az-digital/az_quickstart": {
          "test https://github.com/az-digital/az_quickstart/pull/1834" : "https://patch-diff.githubusercontent.com/raw/az-digital/az_quickstart/pull/1834.diff"
    }
  }
}

@trackleft
Copy link
Member

trackleft commented Aug 17, 2022

Have we tested that this works for existing sites?

@trackleft
Copy link
Member

image

@trackleft
Copy link
Member

Quickstart site with update looks like this
image

@trackleft
Copy link
Member

BEFORE SCREENSHOT
image

@trackleft
Copy link
Member

trackleft commented Aug 17, 2022

In case it isn't totally obvious to everyone, the title block bottom margin combined with the sidebar top margin do bring the sidebar out of alignment with the body content on that particular site.

@akslay
Copy link
Contributor Author

akslay commented Aug 17, 2022

In case it isn't totally obvious to everyone, the title block bottom margin combined with the sidebar top margin do bring the sidebar out of alignment with the body content on that particular site.

I think that'll be a non-issue once I update #1835 and we get it merged; that PR will remove the sidebar margin-top.

@trackleft
Copy link
Member

Cool, just didn't want it to get overlooked.

@trackleft
Copy link
Member

trackleft commented Aug 17, 2022

Blocked or should be merged together into main with #1835

@camikazegreen camikazegreen removed the high priority Must get done for this milestone label Jan 6, 2023
@danahertzberg
Copy link
Contributor

Could consider adding a new region for page title that uses the display flex classes. Float right when sidebar is next to it, but fill the whole column when using full width elements at the top of a page (below the title).

@HenryzGreenberg
Copy link

Thank you Ashley for your time today. I do see value in moving the page title block to the content_top region until we have a solution for mobile navigation that address the multiple levels of navigation question.

@akslay akslay assigned danahertzberg and unassigned akslay May 31, 2023
@joeparsons joeparsons added needs discussion Further discussion required to determine requirements proposal Proposed change to how something works (usually larger or more fundamental than a feature request) and removed 2.7.x only labels Aug 4, 2023
@joeparsons
Copy link
Member

Decided in 2023-08-04 Friday meeting to close this for now. We can definitely re-open/re-create this if needed later.

@joeparsons joeparsons closed this Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Further discussion required to determine requirements proposal Proposed change to how something works (usually larger or more fundamental than a feature request) visual change Introduces a visual change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve page title location
7 participants