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

Sort files and folders in the share path #1960

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Sort files and folders in the share path #1960

merged 4 commits into from
Jul 29, 2020

Conversation

efcy
Copy link
Contributor

@efcy efcy commented Jul 29, 2020

Motivation and context

Currently the files and folders lists under share path in the task creation view are not sorted. This can be annoying to work with when creating tasks manually for a lot of folders.

How has this been tested?

Added a path as share path with a lot of sub folders and checked manually that those folders are sorted alphabetically.

Checklist

  • I submit my changes into the develop branch
  • I have added description of my changes into CHANGELOG file
  • I have updated the documentation accordingly
    - [ ] I have added tests to cover my changes
  • I have linked related issues (read github docs) -> have not found related issues.
  • I have increased versions of npm packages if it is necessary (cvat-canvas,
    cvat-core, cvat-data and cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
    - [ ] I have updated the license header for each file (see an example below)
# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

@efcy efcy requested a review from bsekachev as a code owner July 29, 2020 10:30
@efcy
Copy link
Contributor Author

efcy commented Jul 29, 2020

@bsekachev I am not sure if I should update this change as a patch or a minor version. I am also not sure if should mention this small change in the documentation. Do you have input regarding this?

@efcy efcy requested a review from nmanovic as a code owner July 29, 2020 10:34
@coveralls
Copy link

coveralls commented Jul 29, 2020

Pull Request Test Coverage Report for Build 6655

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 64.988%

Totals Coverage Status
Change from base Build 6649: 0.004%
Covered Lines: 11098
Relevant Lines: 16675

💛 - Coveralls

@bsekachev
Copy link
Member

@BenjiSchlotter

Hi, please patch version of cvat-ui.
No need to mention it in the documentation.
Thx.

@@ -129,6 +129,8 @@ export default class FileManager extends React.PureComponent<Props, State> {

private renderShareSelector(): JSX.Element {
function renderTreeNodes(data: TreeNodeNormal[]): JSX.Element[] {
// sort alphabetically
data.sort((a, b) => (a.key > b.key) ? 1 : -1)
Copy link
Member

Choose a reason for hiding this comment

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

Static linter shows the issue here: Arrow function used ambiguously with a conditional expression.eslintno-confusing-arrow

Maybe use such kind of sorting?
data.sort((a: TreeNodeNormal, b: TreeNodeNormal): number => a.key.localeCompare(b.key));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to your suggestion.

@bsekachev bsekachev merged commit 0de65e7 into cvat-ai:develop Jul 29, 2020
@bsekachev
Copy link
Member

@BenjiSchlotter

Thanks again for the contribution

@efcy efcy deleted the pr/sort_share_folder branch July 29, 2020 11:40
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.

3 participants