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

[UNDERTOW-2404] Add default sorting by type and name in directory lis… #1660

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

xjusko
Copy link
Contributor

@xjusko xjusko commented Sep 3, 2024

…ting view; enable clickable name and size column headers for custom sorting
https://issues.redhat.com/browse/UNDERTOW-2404

@baranowb baranowb added enhancement Enhances existing behaviour or code under verification Currently being verified (running tests, reviewing) before posting a review to contributor new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) waiting peer review PRs that edit core classes might require an extra review labels Sep 4, 2024

int state = 0;
String parent = getParentPath(path, state);

SimpleDateFormat format = new SimpleDateFormat("MMM dd, yyyy HH:mm:ss", Locale.US);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Its an enhancement, lets improve and ditch imperials( or possibly Locale.getDefault() and make decision based on this info?

Comparator<Resource> comparator;
if ("lastModified".equals(sortColumn)) {
comparator = Comparator.comparing(
entry -> (entry.getLastModified() == null) ? new Date(0L) : entry.getLastModified()
Copy link
Contributor

Choose a reason for hiding this comment

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

How this can be null?

Copy link
Contributor

Choose a reason for hiding this comment

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

if its somehow possible, maybe just use creation date rather than 1970 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no creation date, but I think using a string like Unknown or Not Available. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

It makes sense to me. As we sure that it is possible to get a null here? Maybe we even want to have something like: "-"
Such as: last modified: -

Comparator<Resource> comparator;
if ("lastModified".equals(sortColumn)) {
comparator = Comparator.comparing(
entry -> (entry.getLastModified() == null) ? new Date(0L) : entry.getLastModified()
Copy link
Member

Choose a reason for hiding this comment

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

It makes sense to me. As we sure that it is possible to get a null here? Maybe we even want to have something like: "-"
Such as: last modified: -

@fl4via fl4via removed the waiting peer review PRs that edit core classes might require an extra review label Sep 18, 2024
@xjusko xjusko force-pushed the UNDERTOW-2404 branch 2 times, most recently from eb1c957 to 6b4575f Compare September 19, 2024 14:02
…ting view; enable clickable name and size column headers for custom sorting
@fl4via fl4via requested a review from baranowb October 4, 2024 11:43
@fl4via fl4via removed the under verification Currently being verified (running tests, reviewing) before posting a review to contributor label Oct 21, 2024
@fl4via fl4via merged commit 4f308ec into undertow-io:main Oct 21, 2024
11 checks passed
@baranowb baranowb added the next release This PR will be merged before next release or has already been merged (for payload double check) label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances existing behaviour or code new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) next release This PR will be merged before next release or has already been merged (for payload double check)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants