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

Refine hover / focus styles for title field’s comment button #9202

Open
thibaudcolas opened this issue Sep 14, 2022 · 8 comments
Open

Refine hover / focus styles for title field’s comment button #9202

thibaudcolas opened this issue Sep 14, 2022 · 8 comments
Assignees
Labels
component:Design system Including the pattern library (Storybook) type:Enhancement

Comments

@thibaudcolas
Copy link
Member

thibaudcolas commented Sep 14, 2022

As of Wagtail 4, our title field now has a border that only reveals on hover. It also has a "Add comment" button revealed on hover. Both elements are currently out of sync – if the user hovers / focuses the comment button, the border doesn’t show:

Title field hover / focus recording

It would be nice if those were always in sync, similarly to what #9164 did for the choosers.

Looking at the styles,

&:not(:hover, :focus, [aria-invalid='true']) {
border-color: transparent;
}

I believe this would need to change to applying the styles based on hover / focus within the parent element. Something like (untested):

// Styles for the title field when at the top of the page.
// It’s the first panel when comments are disabled, second if enabled.
.w-panel.title:nth-child(1),
.w-panel.title:nth-child(2) {
  .w-panel__header {
    @apply w-sr-only;

    .w-panel__anchor,
    .w-panel__toggle {
      @apply w-hidden;
    }
  }

  input,
  textarea,
  .Draftail-Editor .public-DraftEditor-content {
    @apply w-h1;
    color: $color-input-text;
    // Slightly out-dented so the field’s is horizontally aligned.
    padding-inline-start: theme('spacing[1.5]');
    margin-inline-start: calc(-1 * theme('spacing[1.5]'));

    &:not([aria-invalid='true']) {
      border-color: transparent;
    }
  }

    &:not(:hover, :focus-within) :is(input, textarea, .Draftail-Editor .public-DraftEditor-content) {
      border-color: transparent;
    }
}

We’d need to make sure this looks correct when hovering/focusing title fields with errors, and tweak if needed.

@alvinokafor
Copy link

Hi @thibaudcolas, I am an outreachy intern an di would love to take this issue up

@osujipius
Copy link

Hi.. @alvinokafor I am currently working on this

@thibaudcolas thibaudcolas added the Outreachy https://www.wagtail.org/outreachy/ label Oct 12, 2022
@thibaudcolas
Copy link
Member Author

@osujipius in the future make sure to claim a task before someone else does! And @alvinokafor, no need to ask, just say you’re taking it on and let us know how you get on.

@Buduzz
Copy link

Buduzz commented Oct 28, 2022

Hi @thibaudcolas, has any PR been created for this issue? If no, I would like to jump on it and get it done as soon as possible

@lb-
Copy link
Member

lb- commented Nov 22, 2022

Is anyone actively working on this issue?

@alvinokafor @osujipius or @Buduzz

@thibaudcolas thibaudcolas removed the Outreachy https://www.wagtail.org/outreachy/ label Nov 22, 2022
@Julietadeboye
Copy link
Contributor

Hi LB, I'll work on this issue. Thanks

@lb-
Copy link
Member

lb- commented Nov 28, 2022

Thanks @Julietadeboye - assigned to you for now.

@Julietadeboye
Copy link
Contributor

Screenshot (488)
This issue seems to have been fixed already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Design system Including the pattern library (Storybook) type:Enhancement
Projects
Status: No status
Status: To prioritise
Development

No branches or pull requests

6 participants