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

Replace custom tooltips with native one for files sharing #35785

Merged
merged 1 commit into from
Dec 23, 2022
Merged

Replace custom tooltips with native one for files sharing #35785

merged 1 commit into from
Dec 23, 2022

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Dec 15, 2022 •
edited
Loading

Resolves only js/jQuery part of: #35654

Fixes #35654

Summary

  • Replace custom tooltips with native ones in js files for files_sharing

Checklist

@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Dec 16, 2022
@JuliaKirschenheuter JuliaKirschenheuter added the 3. to review Waiting for reviews label Dec 16, 2022
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review December 16, 2022 07:52
@JuliaKirschenheuter
Copy link
Contributor Author

I was not able to test:

https://localhost/apps/files/?dir=/&view=deletedshares
https://localhost/apps/files/?dir=/&view=pendingshares

Reviewers: please help me with it

@ChristophWurst
Copy link
Member

Reviewers: please help me with it

Could you tell us what to test exactly?

ChristophWurst
ChristophWurst requested changes Dec 16, 2022
View reviewed changes
apps/files_sharing/js/files_drop.js Outdated Show resolved Hide resolved
apps/files_sharing/js/files_drop.js Outdated Show resolved Hide resolved
apps/files_sharing/src/share.js Outdated Show resolved Hide resolved
PVince81
PVince81 approved these changes Dec 16, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 requested review from Pytal and artonge December 16, 2022 10:37
ShGKme
ShGKme reviewed Dec 21, 2022
View reviewed changes
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

It looks good to me.
I have checked that .tooltip() method is not used anywhere else.

It doesn't brakes anything, but maybe also remove data-toggle="tooltip" attribute from HTML templates. This attr is not used now.

apps/files_sharing/js/files_drop.js Show resolved Hide resolved
ShGKme
ShGKme approved these changes Dec 21, 2022
View reviewed changes
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

👍

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/35654-js-files/replace_custom_tooltips_with_native_one_for_files_sharing branch from 2434824 to e36ce00 Compare December 22, 2022 07:48
@JuliaKirschenheuter JuliaKirschenheuter added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 22, 2022
PVince81
PVince81 approved these changes Dec 22, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Member

@JuliaKirschenheuter in this specific case, please recompile by running ./build/compile-handlebars-templates.sh
it's a rare case because we usually don't touch the legacy handlebars templates

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/35654-js-files/replace_custom_tooltips_with_native_one_for_files_sharing branch from e36ce00 to a67c53a Compare December 22, 2022 13:00
@JuliaKirschenheuter JuliaKirschenheuter merged commit 99382a0 into master Dec 23, 2022
@JuliaKirschenheuter JuliaKirschenheuter deleted the enh/35654-js-files/replace_custom_tooltips_with_native_one_for_files_sharing branch December 23, 2022 09:07
@ChristophWurst
Copy link
Member

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Dec 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShGKme ShGKme ShGKme approved these changes

@PVince81 PVince81 PVince81 approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@Pytal Pytal Awaiting requested review from Pytal

@artonge artonge Awaiting requested review from artonge

Assignees

@JuliaKirschenheuter JuliaKirschenheuter

Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BITV]: Replace custom tooltips with native one for files_sharing
4 participants
@JuliaKirschenheuter @ChristophWurst @PVince81 @ShGKme