Skip to content

mobile-tabs - Restore feature#9231

Closed
Copilot wants to merge 7 commits into
mainfrom
copilot/fix-mobile-tabs-selectors
Closed

mobile-tabs - Restore feature#9231
Copilot wants to merge 7 commits into
mainfrom
copilot/fix-mobile-tabs-selectors

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026 •
edited by fregante
Loading

No description provided.

Copilot AI linked an issue Apr 20, 2026 that may be closed by this pull request
mobile-tabs broken #9070
Closed
1 task
@fregante fregante changed the title [WIP] Fix selectors in mobile-tabs.css for new DOM structure mobile-tabs - Restore feature Apr 20, 2026
@fregante fregante added the bug label Apr 20, 2026
Copilot AI changed the title mobile-tabs - Restore feature Update mobile-tabs selectors for GitHub’s new repository nav markup Apr 20, 2026
Copilot AI requested a review from fregante April 20, 2026 16:38
@fregante fregante changed the title Update mobile-tabs selectors for GitHub’s new repository nav markup mobile-tabs - Restore feature Apr 20, 2026
fregante
fregante requested changes Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

  • li > a doesn't fill li vertically, li has this style: 
    .prc-UnderlineNav-UnderlineNavItem-syRjR {
  flex-direction: column;
  align-items: center;
  display: flex;
}
  • the contents of li > a should be bottom-aligned
  • the data-tab-item="security-and-quality" tab has a long label. shorten to "Sec" like we did for Pulls
  • the custom "Pulls" label is bold, but it shouldn't be bold anymore
  • some tabs are missing the counter, so the top-right cell is empty; refer to the previous CSS to target this format again, it will center the icon horizontally. We already have CSS that does this but the selectors are out of date.
  • Some tabs are missing the icon, the same thing should happen
  • The icon and counter should have margin:0!important
  • Don't rename the PR

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 20, 2026

  • li > a doesn't fill li vertically, li has this style: 
    .prc-UnderlineNav-UnderlineNavItem-syRjR {...

Fixed in 687c947. I updated the mobile tab CSS to bottom-align li > a content and make links fill height, added Security and quality → Sec, removed bold pseudo-label text, restored missing-counter/missing-icon centering behavior with updated selectors, and set icon/counter margins to 0 !important. Screenshot: https://github.com/user-attachments/assets/f436a62c-3244-4281-ada0-aea06afb9ff2

Copilot AI requested a review from fregante April 20, 2026 16:49
@fregante fregante marked this pull request as ready for review April 20, 2026 16:51
fregante
fregante requested changes Apr 20, 2026
View reviewed changes
Comment thread source/features/mobile-tabs.css Outdated
Comment thread source/features/mobile-tabs.css Outdated
Comment thread source/features/mobile-tabs.css Outdated
@fregante fregante marked this pull request as draft April 20, 2026 17:12
@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 20, 2026 •
edited
Loading

Here's what I got so far:

Screenshot 2

The icons are completely removed from the DOM via React under 609 pixels, so this is the best we can do:

Screenshot 3

@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 20, 2026

Can't finish this tonight. Releasing 26.4.20 without it. I can add it via hotfix later.

Copilot AI requested a review from fregante April 20, 2026 17:17
@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 24, 2026

They have some weird on-the-fly calculation and caching that break any attempts at changing the size. Mainly:

  • the icons are dropped at some random resolution, calculated via JS
  • the More dropdown doesn't appear at the right time, it depends on whether you start on a small viewport or not, but the calculation is off anyway

I'm going back to the idea we should reimplement the tabs from scratch, but this is super risky for future developments by GitHub. The really difficult part is knowing what tabs are available: GitHub won't render them at all if the viewport is too small :(

@fregante fregante closed this Apr 24, 2026
@fregante fregante deleted the copilot/fix-mobile-tabs-selectors branch April 24, 2026 07:52
This was referenced Apr 24, 2026
Merged
Closed
@SunsetTechuila
Copy link
Copy Markdown
Member

SunsetTechuila commented Apr 24, 2026

calculated via JS

https://github.com/primer/react/blob/d813099b7377be4eed329b4276e8b231054326c8/packages/react/src/UnderlineNav/UnderlineNav.tsx

https://github.com/primer/react/blob/d813099b7377be4eed329b4276e8b231054326c8/packages/react/src/UnderlineNav/UnderlineNavItem.tsx#L81-L103

@fregante fregante mentioned this pull request Apr 25, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fregante fregante Awaiting requested review from fregante

Assignees

@fregante fregante

Copilot code review Copilot

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

mobile-tabs broken

3 participants

@fregante @SunsetTechuila