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

fix(translations/dashboard): fix table sorting #9910

Merged
merged 11 commits into from
Apr 19, 2024

Conversation

leon-win
Copy link
Member

Problem

Main problem: in table "List of direct subpages" only 4 columns supports sorting ("Slug", "Popularity rank (en-US", "Popularity rank ({locale}" and "Date delta"). If user clicks on any other column, an error occurs that can only be resolved by clearing the browser data.

Secondary: active (sorted) column header looked unreadable when the dark theme was enabled (see screenshots). Also indicating of reverse sorting order implemented by flipping the text upside down, this is a fun solution))) but not very user-friendly because it's hard to read the column title when it's upside down.

Solution

Main problem: add sortable prop for columns that support sorting. It also helps to style sortable column headers (use custom cursor).

Secondary: replace hard-coded colors with variables and add icon 'small-arrow` (I haven't found a more suitable icon) instead of flipping the column title.


Screenshots

Before

before-dark
before-light-reverse

After

after-dark
after-light-reverse

@leon-win leon-win requested a review from a team as a code owner October 30, 2023 23:40
@leon-win leon-win force-pushed the fix/translations-dashboard-column-sorting branch from 795cc5b to 64e78b8 Compare November 7, 2023 18:59
@leon-win
Copy link
Member Author

@peterbe, @SphinxKnight, @caugner, help me with review please 🙏

@SphinxKnight
Copy link
Member

Really sorry for the silence here @leon-win , I'll try to have a look in the coming days

cc @mdn/core-dev

@github-actions github-actions bot added the idle label Feb 12, 2024
@github-actions github-actions bot removed the idle label Feb 14, 2024
@github-actions github-actions bot added the idle label Mar 16, 2024
@github-actions github-actions bot removed the idle label Mar 19, 2024
@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Apr 4, 2024
Copy link
Contributor

github-actions bot commented Apr 4, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

@caugner
Copy link
Contributor

caugner commented Apr 4, 2024

@leon-win We merged some changes to the translations/differences page. Can you please take a look and resolve the merge conflicts? 🙏

@hochan222 @queengooborg Would you mind reviewing and testing this change locally? That would be awesome.

@caugner caugner marked this pull request as draft April 4, 2024 12:24
@github-actions github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Apr 4, 2024
@leon-win
Copy link
Member Author

leon-win commented Apr 4, 2024

@leon-win We merged some changes to the translations/differences page. Can you please take a look and resolve the merge conflicts? 🙏

@hochan222 @queengooborg Would you mind reviewing and testing this change locally? That would be awesome.

Conflicts resolved 👌

I also changed some the styles a little to make the content on the pages look better (indentation and stretching to fit the width).

Before (in main branch now):
image
image

After:
image
image

@leon-win leon-win marked this pull request as ready for review April 5, 2024 10:36
Copy link
Member

@LeoMcA LeoMcA left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@LeoMcA LeoMcA merged commit 2cd44e4 into mdn:main Apr 19, 2024
10 checks passed
@leon-win leon-win deleted the fix/translations-dashboard-column-sorting branch April 19, 2024 17:05
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.

4 participants