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: focus issue on translate modal, remove duplicate code #5630

Merged
merged 2 commits into from
Apr 26, 2024
Merged

fix: focus issue on translate modal, remove duplicate code #5630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ca45b2
fix: focus issue on translate modal, remove duplicate code
luka-nextcloud Apr 5, 2024
9e5b885
fix: focus issue on translate modal, remove duplicate code
luka-nextcloud Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions cypress/e2e/Assistant.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,28 @@ describe('Assistant', () => {
.should('be.visible')
.should('contain', 'Hello World')
})

it('Open translate dialog', () => {
cy.isolateTest({
sourceFile: fileName,
})
cy.openFile(fileName, { force: true })

cy.getContent()
.click({ force: true })
cy.get('[data-cy="assistantMenu"]')
.click()
cy.get('[data-cy="open-translate"]')
.should('be.visible')
.click()

cy.get('[data-cy="translate-input"]')
.should('be.visible')
.click()
cy.get('[data-cy="translate-input"]')
.should('be.visible')
.type('Hello World')
cy.get('[data-cy="translate-input"]')
.should('be.focused')
})
})
2 changes: 1 addition & 1 deletion cypress/e2e/share.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('Open test.md in viewer', function() {
cy.getModal().getContent().should('contain', 'Hello world')
cy.getModal().getContent().find('h2').should('contain', 'Hello world')
cy.getModal().find('.modal-header button.header-close').click()
cy.get('.modal-mask').should('not.exist')
cy.get('.modal-mask').should('not.be.visible')
// cy.get('#rich-workspace').getContent().should('contain', 'Hello world')
})
})
Expand Down
38 changes: 6 additions & 32 deletions src/components/Assistant.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
</template>
{{ provider.name }}
</NcActionButton>
<NcActionButton close-after-click @click="openTranslateDialog">
<NcActionButton data-cy="open-translate" close-after-click @click="openTranslateDialog">
<template #icon>
<TranslateVariant :size="20" />
</template>
Expand All @@ -63,12 +63,6 @@
class="floating-menu--badge" />
</FloatingMenu>

<Translate v-if="displayTranslate !== false"
:content="displayTranslate"
@insert-content="translateInsert"
@replace-content="translateReplace"
@close="hideTranslate" />

<NcModal :show.sync="showTaskList">
<div class="task-list">
<h4 v-if="tasks.length > 0">
Expand Down Expand Up @@ -154,8 +148,7 @@ import {
useIsPublicMixin,
} from './Editor.provider.js'
import { FloatingMenu } from '@tiptap/vue-2'
import Translate from './Modal/Translate.vue'
import { subscribe, unsubscribe } from '@nextcloud/event-bus'
import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'

const limitInRange = (num, min, max) => {
return Math.min(Math.max(parseInt(num), parseInt(min)), parseInt(max))
Expand All @@ -170,7 +163,6 @@ const STATUS_UNKNOWN = 0
export default {
name: 'Assistant',
components: {
Translate,
FloatingMenu,
ErrorIcon,
CreationIcon,
Expand Down Expand Up @@ -209,7 +201,6 @@ export default {
STATUS_UNKNOWN,

showTaskList: false,
displayTranslate: false,
canTranslate: loadState('text', 'translation_languages', []).length > 0,
}
},
Expand Down Expand Up @@ -304,27 +295,10 @@ export default {
await this.fetchTasks()
},
openTranslateDialog() {
this.displayTranslate = this.selection
},
hideTranslate() {
this.displayTranslate = false
},
translateInsert(content) {
this.$editor.commands.command(({ tr, commands }) => {
return commands.insertContentAt(tr.selection.to, content)
})
this.displayTranslate = false
},
translateReplace(content) {
this.$editor.commands.command(({ tr, commands }) => {
const selection = tr.selection
const range = {
from: selection.from,
to: selection.to,
}
return commands.insertContentAt(range, content)
})
this.displayTranslate = false
if (!this.selection.trim().length) {
this.$editor.commands.selectAll()
}
emit('text:translate-modal:show', { content: this.selection || '' })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this even make sense if the selection is empty?
The previous code looks as if it did not open the modal on an empty selection:

			this.displayTranslate = this.selection

However I just tried it on cloud.nextcloud.com and there it opened anyway but did not work (due to the broken input but also as it had no content).

Copy link
Contributor Author

@luka-nextcloud luka-nextcloud Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@max-nextcloud

Does this even make sense if the selection is empty?

The modal should be open anyway. User is free to update the input text.
If selection is empty, all content will be selected. I updated this behavior to same as action on menu bar.

PR updated:

  • select all if selection is empty when open translate dialog
  • add cypress test

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the $editor.selectAll have some sideeffects? Is the text selected fully afterwards? Otherwise this sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@max-nextcloud I didn't find any side effects. Yes, the text is selected fully.

},
async openResult(task) {
window.OCA?.TPAssistant.openAssistantResult(task)
Expand Down
36 changes: 36 additions & 0 deletions src/components/Editor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@
:is-rich-editor="isRichEditor" />
</Wrapper>
<Assistant v-if="$editor" />
<Translate :show="translateModal"
:content="translateContent"
@insert-content="translateInsert"
@replace-content="translateReplace"
@close="hideTranslate" />
</div>
</template>

Expand Down Expand Up @@ -125,6 +130,7 @@ import MainContainer from './Editor/MainContainer.vue'
import Wrapper from './Editor/Wrapper.vue'
import SkeletonLoading from './SkeletonLoading.vue'
import Assistant from './Assistant.vue'
import Translate from './Modal/Translate.vue'

export default {
name: 'Editor',
Expand All @@ -139,6 +145,7 @@ export default {
Reader: () => import(/* webpackChunkName: "editor" */'./Reader.vue'),
Status,
Assistant,
Translate,
},
mixins: [
isMobile,
Expand Down Expand Up @@ -251,6 +258,8 @@ export default {
draggedOver: false,

contentWrapper: null,
translateModal: false,
translateContent: '',
}
},
computed: {
Expand Down Expand Up @@ -338,6 +347,7 @@ export default {
const maxWidth = width - 36
this.$el.style.setProperty('--widget-full-width', `${maxWidth}px`)
})
subscribe('text:translate-modal:show', this.showTranslateModal)
},
created() {
this.$ydoc = new Doc()
Expand All @@ -364,6 +374,7 @@ export default {
await Promise.any([timeout, this.$syncService.save()])
}
this.$providers.forEach(p => p.destroy())
unsubscribe('text:translate-modal:show', this.showTranslateModal)
},
methods: {
initSession() {
Expand Down Expand Up @@ -757,6 +768,31 @@ export default {
event.preventDefault()
}
},

showTranslateModal(e) {
this.translateContent = e.content
this.translateModal = true
},
hideTranslate() {
this.translateModal = false
},
translateInsert(content) {
this.$editor.commands.command(({ tr, commands }) => {
return commands.insertContentAt(tr.selection.to, content)
})
this.translateModal = false
},
translateReplace(content) {
this.$editor.commands.command(({ tr, commands }) => {
const selection = tr.selection
const range = {
from: selection.from,
to: selection.to,
}
return commands.insertContentAt(range, content)
})
this.translateModal = false
},
},
}
</script>
Expand Down
33 changes: 3 additions & 30 deletions src/components/Menu/MenuBar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@
'text-menubar--is-workspace': $isRichWorkspace
}">
<HelpModal v-if="displayHelp" @close="hideHelp" />
<Translate v-if="displayTranslate !== false"
:content="displayTranslate"
@insert-content="translateInsert"
@replace-content="translateReplace"
@close="hideTranslate" />

<div v-if="$isRichEditor"
ref="menubar"
Expand All @@ -63,7 +58,7 @@
:can-be-focussed="activeMenuEntry === visibleEntries.length"
@click="activeMenuEntry = 'remain'">
<template #lastAction="{ visible }">
<NcActionButton v-if="canTranslate" @click="showTranslate">
<NcActionButton v-if="canTranslate" close-after-click @click="showTranslate">
<template #icon>
<TranslateVariant />
</template>
Expand All @@ -85,14 +80,14 @@
import { NcActionSeparator, NcActionButton } from '@nextcloud/vue'
import { loadState } from '@nextcloud/initial-state'
import { useResizeObserver } from '@vueuse/core'
import { emit } from '@nextcloud/event-bus'

import ActionFormattingHelp from './ActionFormattingHelp.vue'
import ActionList from './ActionList.vue'
import ActionSingle from './ActionSingle.vue'
import CharacterCount from './CharacterCount.vue'
import HelpModal from '../HelpModal.vue'
import ToolBarLogic from './ToolBarLogic.js'
import Translate from './../Modal/Translate.vue'
import actionsFullEntries from './entries.js'
import { MENU_ID } from './MenuBar.provider.js'
import { DotsHorizontal, TranslateVariant } from '../icons.js'
Expand All @@ -114,7 +109,6 @@ export default {
NcActionButton,
CharacterCount,
TranslateVariant,
Translate,
},
extends: ToolBarLogic,
mixins: [
Expand Down Expand Up @@ -145,7 +139,6 @@ export default {
entries: [...actionsFullEntries],
randomID: `menu-bar-${(Math.ceil((Math.random() * 10000) + 500)).toString(16)}`,
displayHelp: false,
displayTranslate: false,
isReady: false,
canTranslate: loadState('text', 'translation_languages', []).length > 0,
resize: null,
Expand Down Expand Up @@ -232,27 +225,7 @@ export default {
}

console.debug('translation click', this.$editor.view.state.selection, selectedText)
this.displayTranslate = selectedText ?? ''
},
hideTranslate() {
this.displayTranslate = false
},
translateInsert(content) {
this.$editor.commands.command(({ tr, commands }) => {
return commands.insertContentAt(tr.selection.to, content)
})
this.displayTranslate = false
},
translateReplace(content) {
this.$editor.commands.command(({ tr, commands }) => {
const selection = tr.selection
const range = {
from: selection.from,
to: selection.to,
}
return commands.insertContentAt(range, content)
})
this.displayTranslate = false
emit('text:translate-modal:show', { content: selectedText })
},
},
}
Expand Down
11 changes: 10 additions & 1 deletion src/components/Modal/Translate.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
-->

<template>
<NcModal size="large" @close="$emit('close')">
<NcModal :show="show" size="large" @close="$emit('close')">
<div class="translate-dialog">
<h2>{{ t('text', 'Translate') }}</h2>
<em>{{ t('text', 'To translate individual parts of the text, select it before using the translate function.') }}</em>
Expand All @@ -39,6 +39,7 @@
:value.sync="input"
:label="t('text', 'Text to translate from')"
autofocus
data-cy="translate-input"
input-class="translate-textarea"
resize="none"
@focus="onInputFocus" />
Expand Down Expand Up @@ -106,6 +107,10 @@ export default {
useIsMobileMixin,
],
props: {
show: {
type: Boolean,
default: false,
},
content: {
type: String,
default: '',
Expand Down Expand Up @@ -165,6 +170,9 @@ export default {
},
},
watch: {
content() {
this.input = this.content
},
input() {
this.result = null
this.error = null
Expand Down Expand Up @@ -246,6 +254,7 @@ export default {
resize: none;
box-sizing: border-box;
overflow-y: auto;
min-height: 62px;
max-height: 58vh;
}
}
Expand Down
Loading