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.

Sign up for GitHub

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

Jump to bottom

fix: Open transferDetail in a sheet #90

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

fix: Open transferDetail in a sheet #90

wants to merge 18 commits into from

Conversation

Ambrdctr
Copy link
Contributor

No description provided.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 17, 2024 •
edited
Loading

Found 4 unused code occurences

Expand 
SwissTransferCoreUI/Components/LargeFileCell.swift:21:1: warning: Imported module 'STResources' is unused
SwissTransferFeatures/ReceivedView/ReceivedView.swift:23:1: warning: Imported module 'SwissTransferCore' is unused
SwissTransferFeatures/SentView/SentView.swift:23:1: warning: Imported module 'SwissTransferCore' is unused
SwissTransferFeatures/TransferDetailsView/ContentView.swift:28:17: warning: Property 'columns' is unused

@valentinperignon valentinperignon self-requested a review December 17, 2024 13:36
@Ambrdctr Ambrdctr force-pushed the folder branch 3 times, most recently from fc13a75 to 4bd0791 Compare January 13, 2025 14:03
adrien-coye
adrien-coye reviewed Jan 13, 2025
View reviewed changes
Copy link
Contributor

@adrien-coye adrien-coye left a comment

Choose a reason for hiding this comment

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

Left some small comments, lemme know what you think.

SwissTransferCore/Array+Extension.swift Show resolved Hide resolved
SwissTransferFeatures/TransferDetailsView/FileListView.swift Outdated Show resolved Hide resolved
SwissTransferFeatures/TransferDetailsView/TransferDetailsView.swift Outdated Show resolved Hide resolved
adrien-coye
adrien-coye reviewed Jan 14, 2025
View reviewed changes
Copy link
Contributor

@adrien-coye adrien-coye left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. 👍 Maybe since this is a UI heavy PR and a did a quick check, @valentinperignon can give the final validation.

@Ambrdctr Ambrdctr force-pushed the folder branch 2 times, most recently from 54baa28 to 820f35a Compare January 15, 2025 07:17
@valentinperignon valentinperignon removed their request for review January 15, 2025 12:08
PhilippeWeidmann
PhilippeWeidmann requested changes Jan 16, 2025
View reviewed changes
Copy link
Member

@PhilippeWeidmann PhilippeWeidmann left a comment

Choose a reason for hiding this comment

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

The FileGridView doesn't contain a DownloadableFileCellView, so we can't preview a file anywhere other than at the root of the TransferDetailsView.

Also, in the TransferDetailsView->ContentView. The FileGridView should be reused to avoid duplication of the grid.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrien-coye adrien-coye adrien-coye left review comments

@PhilippeWeidmann PhilippeWeidmann PhilippeWeidmann requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@Ambrdctr Ambrdctr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ambrdctr @PhilippeWeidmann @adrien-coye