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

Add ability to copy internal link from share dialog #4162

Merged
merged 1 commit into from
Jan 24, 2022
Merged

Add ability to copy internal link from share dialog #4162

merged 1 commit into from
Jan 24, 2022

Conversation

claucambra
Copy link
Collaborator

This addresses #815 and helps replicate the functionality available on the web interface

image

@claucambra claucambra self-assigned this Jan 10, 2022
@claucambra claucambra force-pushed the feature/internallink branch 3 times, most recently from 15654ec to 9e006d5 Compare January 10, 2022 13:48
camilasan
camilasan requested changes Jan 11, 2022
View reviewed changes
@camilasan
Copy link
Member

Locally I can not compile it. I get the same error as SonarCloud.

@claucambra claucambra force-pushed the feature/internallink branch from 0d0ba63 to 9e006d5 Compare January 11, 2022 16:13
@claucambra
Copy link
Collaborator Author

Locally I can not compile it. I get the same error as SonarCloud.

Made a bad use of QScopedPointer, fixed now :)

@claucambra claucambra requested a review from camilasan January 11, 2022 16:27
@claucambra claucambra force-pushed the feature/internallink branch 2 times, most recently from 22b61e2 to eefa0c3 Compare January 11, 2022 16:28
mgallien
mgallien reviewed Jan 12, 2022
View reviewed changes
camilasan
camilasan requested changes Jan 12, 2022
View reviewed changes
@claucambra claucambra force-pushed the feature/internallink branch from 8c89992 to c29152a Compare January 13, 2022 13:21
@mgallien
Copy link
Collaborator

@claucambra can you have a look at the conflict and rebase on your side ?

mgallien
mgallien reviewed Jan 14, 2022
View reviewed changes
@claucambra claucambra force-pushed the feature/internallink branch 2 times, most recently from 6cacbe0 to b3b9808 Compare January 14, 2022 15:04
jancborchardt
jancborchardt requested changes Jan 17, 2022
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

2 things design-wise:

  • The icon we use in server is different from the normal link one to separate them: icon-external
  • The text would look better if the subline is indented the same as the main text, and they are vertically aligned as a unit with the icons (as in the server)

See server for reference:
image

@claucambra claucambra force-pushed the feature/internallink branch from 58c0fa1 to 06c2aa2 Compare January 17, 2022 11:36
@claucambra
Copy link
Collaborator Author

2 things design-wise:

* The icon we use in server is different from the normal link one to separate them: [icon-external](https://docs.nextcloud.com/server/22/developer_manual/design/icons.html#id35)

* The text would look better if the subline is indented the same as the main text, and they are vertically aligned as a unit with the icons (as in the server)

See server for reference: image

I followed your suggestions and tried to make the widget look as close to the server's as possible:

image

jancborchardt
jancborchardt approved these changes Jan 18, 2022
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Much better @claucambra, thank you! :)

mgallien
mgallien requested changes Jan 20, 2022
View reviewed changes
@claucambra claucambra force-pushed the feature/internallink branch from 6cd041d to 2c60770 Compare January 20, 2022 12:03
@claucambra claucambra requested a review from mgallien January 20, 2022 12:03
@mgallien
Copy link
Collaborator

@claucambra the drone check is still failing because there was also another failed static check on code
can you have a look ?

@claucambra
Copy link
Collaborator Author

@claucambra the drone check is still failing because there was also another failed static check on code can you have a look ?

Right, thanks, fixed it

@claucambra
Copy link
Collaborator Author

/rebase

@claucambra claucambra force-pushed the feature/internallink branch 2 times, most recently from f485fac to 2f4e511 Compare January 21, 2022 16:46
@camilasan
Copy link
Member

/rebase

@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-4162-624213956e624c0f7319dae7ea01c121a274135c-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@mgallien mgallien merged commit 41a1949 into master Jan 24, 2022
@mgallien mgallien deleted the feature/internallink branch January 24, 2022 10:39
@mgallien mgallien added this to the 3.5.0 milestone Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@camilasan camilasan camilasan approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

@allexzander allexzander Awaiting requested review from allexzander

Assignees

@claucambra claucambra

Labels
None yet
Projects
None yet
Milestone
3.5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@claucambra @camilasan @mgallien @nextcloud-desktop-bot @jancborchardt