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

Tables: Fix sorting order in demo->tables->sorting #3023

Closed
wants to merge 25 commits into from

Conversation

piotrjurga
Copy link

I found a minor issue in the demo window in tables branch. The sorting order is reversed:
sorting

It was caused by subtracting rhs from lhs in the comparison function, instead of the opposite, as seen in diff.

ocornut and others added 25 commits February 10, 2020 14:03
… are synced.

(some minor one-frame lack of sync when e.g. toggling visibility in context menu)
…idthFixed instead of WidthAlwaysAutoResize if an explicit value is passed to TableSetupColumn()
Return false in user functions, set SkipItems in window, redirect to dummy draw channel.
…ing users.

Fixed some inconsistency with BeginTable/EndTable without row.
Move some of the TableBegin() code in TableBeginUpdateColumns().
Allow to submit multiple header lines.
…le to remove outer vertical borders to mimic old columns.

VInner or VOuter only don't have correct padding/spacing.
…ellRect() include expected paddings. Add demo code. Comments.

Remove misleading commented-out flags for now.
…to never be larger than scrolling visible rect width.
…xtBaseOffset in EndCell of inactive column).
…api) and exposed flags. Added simple Tree demo.
…ad since we don't actually restore that data currently.

Demo: Remove filter from Advanced Table demo since it's breaking with clipping.
…ental one to allow multi-item multi-line in header cells. Demo TableHeader() - will caveat, comments.
… handling of _DefaultSort with _PreferSortXXXflags (@parbo). Comments.
@ocornut
Copy link
Owner

ocornut commented Feb 13, 2020

Hello,
When I designed it I mimicked Windows Explorer behavior and I think it is right as it is?

@piotrjurga
Copy link
Author

piotrjurga commented Feb 13, 2020

I don't think so, unless Windows 10 changed the convention, usually the triangle means "from smallest (peak) to largest (base)". For example here, a ascending lexicographic sort uses ▲:
explorer

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2020

Hello @piotrjurga ,
You are right this somehow got inverted at some point.

I have fixed it now with:
d456e19

  • Your fix wasn't correct as-is as it would affect the initial sort order on clicking a column.
  • The order of the subtraction wascorrect as per qsort standard http://www.cplusplus.com/reference/cstdlib/qsort/
  • I removed SortSign from the sort specs because it was both duplicate from SortDirection (therefore misleading/confusing) and because different sort functions work different this integer value/sign was also more misleading. Better removed!

Thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants