Skip to content

Conversation

@akramcodez
Copy link
Contributor

Closes #11182

This PR ensures that dropdown rows (such as for languages and contributor roles) in forms and autocomplete menus have consistent height and padding across all devices, especially improving usability on touch devices like mobile and tablets. It increases the minimum height and padding on touch screens to make selection easier and more accessible.

Technical

  • Adds @media (pointer: coarse) CSS to relevant selectors in LESS files for forms and autocomplete.
  • Sets consistent min-height: 44px and appropriate padding for dropdown options and autocomplete rows.
  • Applies changes to form selects, .ac_language autocomplete items, and textareas (edit screen).

Testing

Screenshot

Stakeholders

@jimchamp

Copilot AI review requested due to automatic review settings November 17, 2025 14:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves mobile usability by adding touch-friendly styles to dropdown elements and form controls, ensuring consistent minimum heights and padding on devices with coarse pointers (touch screens).

  • Adds @media (pointer: coarse) queries with min-height: 44px to meet touch target accessibility guidelines
  • Applies consistent padding to autocomplete dropdowns, form inputs, and select elements for better touch interaction
  • Targets specific elements: form inputs/selects in page-edit, autocomplete list items and language options, and olform select elements

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
static/css/page-edit.less Adds touch-friendly padding and min-height to form inputs and select elements on touch devices
static/css/components/jquery.autocomplete.less Increases padding and sets min-height for autocomplete dropdown items and language selections on touch devices
static/css/components/form.olform.less Adds touch-specific styles to select elements within olform, but contains a syntax error with missing line break

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@akramcodez
Copy link
Contributor Author

@jimchamp I tried to fix it, but there might be multiple issues. PTAL and suggest how I can fix them.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Nov 18, 2025
@jimchamp
Copy link
Collaborator

jimchamp commented Dec 3, 2025

Thanks @akramcodez.

I can sort of understand why you changed all of the form select inputs. I do not understand why you've changed the textarea styling.

The only thing in scope for this PR is the language auto-complete options. Please revert all other unrelated changes.

@jimchamp jimchamp added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] and removed Needs: Response Issues which require feedback from lead labels Dec 3, 2025
@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Dec 4, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to 53
@media (pointer: coarse) {
padding: 12px 5px;
min-height: 44px;
}
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The padding is being applied to the inner .ac_language div, but the parent li element (line 22) already has padding: 5px. This could result in inconsistent spacing. Consider applying the touch target styles directly to the li element using a selector like .ac_results li:has(.ac_language) or applying the styles to .ac_results li with the media query and removing padding from the inner div to avoid double padding.

Suggested change
@media (pointer: coarse) {
padding: 12px 5px;
min-height: 44px;
}
}
@media (pointer: coarse) {
// Move touch target padding/min-height to parent li to avoid double padding
// Remove padding from .ac_language
}
// Apply touch target styles to li containing .ac_language for coarse pointers
@media (pointer: coarse) {
.ac_results li:has(.ac_language) {
padding: 12px 5px;
min-height: 44px;
}
}
}

Copilot uses AI. Check for mistakes.
@akramcodez
Copy link
Contributor Author

@jimchamp I’m a bit confused here. I’ve reverted the mistakes, should I implement the Copilot suggestion?

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Dec 4, 2025
@jimchamp
Copy link
Collaborator

jimchamp commented Dec 4, 2025

If copilot told you to jump off of a cliff, would you do it?

You fixed the issue. The other autocomplete options are already large enough on smaller screens.

@jimchamp jimchamp merged commit 6304926 into internetarchive:master Dec 4, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Response Issues which require feedback from lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make language dropdown cell height consistent with contributor role dropdown cell height

2 participants