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

fix(files): right click actions menu #43254

Closed
wants to merge 1 commit into from
Closed

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Feb 1, 2024

After #43090

The root changed

const root = this.$root.$el as HTMLElement
const contentRect = root.getBoundingClientRect()
// Using Math.min/max to prevent the menu from going out of the AppContent
// 200 = max width of the menu
root.style.setProperty('--mouse-pos-x', Math.max(contentRect.left, Math.min(event.clientX, event.clientX - 200)) + 'px')
root.style.setProperty('--mouse-pos-y', Math.max(contentRect.top, event.clientY - contentRect.top) + 'px')

@skjnldsv skjnldsv added this to the Nextcloud 29 milestone Feb 1, 2024
@skjnldsv skjnldsv requested review from ShGKme and a team February 1, 2024 09:36
@skjnldsv skjnldsv self-assigned this Feb 1, 2024
@skjnldsv skjnldsv requested review from nfebe, sorbaugh and emoral435 and removed request for a team February 1, 2024 09:36
@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 1, 2024

/backport! to backport/43090/stable28

@skjnldsv skjnldsv changed the title fix(files): right lick actions menu fix(files): right click actions menu Feb 1, 2024
@@ -346,7 +346,7 @@ export default Vue.extend({
<style lang="scss">
// Allow right click to define the position of the menu
// only if defined
.app-content[style*="mouse-pos-x"] .v-popper__popper {
.content[style*="mouse-pos-x"] .v-popper__popper {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it the same? Do we need root class here at all?

Suggested change
.content[style*="mouse-pos-x"] .v-popper__popper {
[style*="mouse-pos-x"] .v-popper__popper {

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't it far less efficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it far less efficient?

Probably it is

Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Overall good - however, it seems that firefox has had yet another regression (😓😓):

left: chrome | right: firefox
firefox_r3qZP3x53L

I remember this being mentioned in a PR before, but I cannot remember for the life of me which PR it was! If you believe this can be ignored for now / chrome is where most people use NC, I can go ahead and submit an approval

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 7, 2024
@Altahrim Altahrim mentioned this pull request Mar 12, 2024
@susnux
Copy link
Contributor

susnux commented Mar 13, 2024

#44139

@susnux susnux closed this Mar 13, 2024
@susnux susnux deleted the fix/right-click-files branch March 13, 2024 11:02
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants