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

Bug: deselecting few witnesses in dropdown then separation line breaks #513

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/components/annotations/variants/VariantItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
:class="[
't-py-2 t-px-2 -t-mx-2 t-mb-1 t-space-x-2 t-rounded-md',
{ 'hover:t-bg-gray-200 dark:hover:t-bg-gray-600 t-cursor-pointer': !isActive },
{ 't-bg-gray-300 dark:t-bg-gray-600 active': isActive},
{ 't-border-b t-border-slate-200 t-rounded-none': showSeparator}]"
{ 't-bg-gray-300 dark:t-bg-gray-600 active': isActive}]"
:data-annotation-id="annotation.id"
@click="handleClick"
>
Expand All @@ -25,7 +24,8 @@
class="t-w-7/12"
v-html="entry"
/>
</div>
</div>
<hr class="t-slate-200 t-my-[6px] t-rounded-none" v-if="showSeparator" data-cy="variant-sep-line"/>

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (18)

Disallow self-closing on HTML void elements (<hr/>)

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (18)

Attribute "v-if" should go before "class"

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (18)

'v-if' should be on a new line

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (18)

'data-cy' should be on a new line

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (20)

Disallow self-closing on HTML void elements (<hr/>)

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (20)

Attribute "v-if" should go before "class"

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (20)

'v-if' should be on a new line

Check warning on line 28 in src/components/annotations/variants/VariantItem.vue

View workflow job for this annotation

GitHub Actions / build (20)

'data-cy' should be on a new line
</template>

<script setup lang="ts">
Expand Down
9 changes: 4 additions & 5 deletions src/components/annotations/variants/VariantsList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
:is-active="isActive(annotation)"
:toggle="toggle"
:witness-color="getWitnessColor(annotation.body.value.witness)"
:show-separator="showLineSeparator(filteredAnnotations, i)"
:show-separator="showLineSeparator(visibleAnnotations, i)"
@select="addAnnotation(annotation.id)"
@unselect="removeAnnotation(annotation.id)"
@show-details="openDetailsDialog"
Expand All @@ -36,7 +36,6 @@ const annotationStore = useAnnotationsStore();


const activeAnnotations = computed<ActiveAnnotation>(() => annotationStore.activeAnnotations);
const filteredAnnotations = computed<Annotation[]>(() => annotationStore.filteredAnnotations);
const visibleAnnotations = computed<Annotation[]>(() => annotationStore.visibleAnnotations);


Expand Down Expand Up @@ -65,9 +64,9 @@ function getWitnessColor(witness: string) {
}


function showLineSeparator(filteredAnnotations, i) {
if (filteredAnnotations[i+1]) {
return Utils.generateTargetSelector(filteredAnnotations[i]) !== Utils.generateTargetSelector(filteredAnnotations[i+1])
function showLineSeparator(visibleAnnotations, i) {
if (visibleAnnotations[i+1]) {
return Utils.generateTargetSelector(visibleAnnotations[i]) !== Utils.generateTargetSelector(visibleAnnotations[i+1])
}
}

Expand Down
80 changes: 61 additions & 19 deletions tests/cypress/e2e/variants.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ const selectors = {
.click()
})

Cypress.Commands.add('checkTargetAnnotationLevel', (targetSelector, annotationLevel) => {
cy.get(selectors.panel3)
.find('#text-content')
.find(targetSelector)
.invoke('attr', 'data-annotation-level')
.should('eq', annotationLevel)
})



Cypress.Commands.add('checkTextInWitnessItemDescription', (witness, description) => {
cy
.contains('h3', witness)
Expand Down Expand Up @@ -68,20 +78,20 @@ const selectors = {

it('Should display third annotation tab', () => {
cy
.get(selectors.tab)
.children()
.eq(2)
.parent()
.should('have.attr', 'data-p-active', 'true')
.should('contain','Variants')
.get(selectors.tab)
.children()
.eq(2)
.parent()
.should('have.attr', 'data-p-active', 'true')
.should('contain','Variants')
});

it('Should show a list of variant items', () => {
cy
.get(selectors.list)
.should('be.visible')
.children()
.should("have.length", 11)
.should("have.length", 13)
});

it('Should switch from an item with no variants to another one with variants and show the list correctly', () => {
Expand All @@ -94,7 +104,7 @@ const selectors = {
.wait(500)
.get(selectors.list)
.children()
.should('have.length', 11)
.should('have.length', 13)
})

it('Should switch normally to the new item when first selecting a few annotations and then switching the item', () => {
Expand Down Expand Up @@ -207,7 +217,7 @@ const selectors = {
.wait(500)
.get(selectors.list)
.children()
.should('have.length', 11)
.should('have.length', 13)
.eq(0).should('contain', 'omisit').and('not.have.class', 'active')
.next().should('contain', 'اللبان').and('not.have.class', 'active')
})
Expand All @@ -232,7 +242,7 @@ const selectors = {
.click()
.get(selectors.list)
.children()
.should('have.length', 11)
.should('have.length', 13)
.eq(0)
.should('not.have.class', 'active')
})
Expand Down Expand Up @@ -294,12 +304,12 @@ const selectors = {
// 2. remove the variant items of this witness from the variants list
.get(selectors.list)
.children()
.eq(3)
.eq(4)
.invoke('attr', 'data-annotation-id')
.should('eq', 'http://ahikar.uni-goettingen.de/ns/annotations/3r14z/annotation-variants-t_Brit_Mus_Add_7209_N1l5l3l5l5l29l4_w_2_1') // means that first annotation item of this target is DFM 614, instead of Cod Arab
.get(selectors.list)
.children()
.eq(6) // expecting that two variant items with Cod Arab 236 were removed, the first variant item of the third target is now 'DFM 614'
.eq(8) // expecting that two variant items with Cod Arab 236 were removed, the first variant item of the third target is now 'DFM 614'
.invoke('attr', 'data-annotation-id')
.should('eq', 'http://ahikar.uni-goettingen.de/ns/annotations/3r14z/annotation-variants-t_Brit_Mus_Add_7209_N1l5l3l5l5l29l4_w_3_1')
})
Expand All @@ -314,16 +324,48 @@ const selectors = {
// effect: add the variant items of this witness in the variants list
.get(selectors.list)
.children()
.eq(3)
.eq(4)
.invoke('attr', 'data-annotation-id')
.should('eq', 'http://ahikar.uni-goettingen.de/ns/annotations/3r14z/annotation-variants-t_Brit_Mus_Add_7209_N1l5l3l5l5l29l4_w_2_0') // means that first annotation item of this group is DFM 614, instead of Cod Arab
.get(selectors.list)
.children()
.eq(7) // expecting that two variant items with Cod Arab 236 were removed, then we aim to access the variant item with witness DFM 614 of the third target with index 6 instead of 8
.eq(9) // expecting that two variant items with Cod Arab 236 were removed, then we aim to access the variant item with witness DFM 614 of the third target with index 6 instead of 8
.invoke('attr', 'data-annotation-id')
.should('eq', 'http://ahikar.uni-goettingen.de/ns/annotations/3r14z/annotation-variants-t_Brit_Mus_Add_7209_N1l5l3l5l5l29l4_w_3_0')
})

it('Should change the highlighting level of the target when all its witnesses are deselected from the drop down', () => {
cy
.clickWitnessItem('4 Witnesses selected', 'DFM 614')
.parent().parent()
.contains('Ming. syr. 258').click()
.parent().parent()
.contains('Sach. 339').click()

// function: check annotation level of target
.checkTargetAnnotationLevel('#t_Brit_Mus_Add_7209_MD17104N1l5l3l7l5l41l2_3', '-1')
.checkTargetAnnotationLevel('#t_Brit_Mus_Add_7209_MD17104N1l5l3l7l5l43l2_2', '0')
.checkTargetAnnotationLevel('#t_Brit_Mus_Add_7209_MD17104N1l5l3l7l5l43l2_3', '0')
})

it('Should show separation line correctly between annotation groups when deselecting witnesses from the witnesses drop down', () => {
cy
.clickWitnessItem('4 Witnesses selected', 'Ming. syr. 258')
.get(selectors.list)
.find('hr[data-cy="variant-sep-line"]')
.should('have.length', 2)
.get(selectors.list)
.children()
.eq(2)
.invoke('attr', 'data-cy')
.should('eq', 'variant-sep-line')
cy.get(selectors.list)
.children()
.eq(6)
.invoke('attr', 'data-cy')
.should('eq', 'variant-sep-line')
})

/*
Commented out this test, since we hide the witnesses details dialog box
it('Should show a dialog box containing a list of a witnesses and a description of them', () => {
Expand Down Expand Up @@ -368,7 +410,7 @@ const selectors = {
// the corresponding variant items should be selected
.get(selectors.list)
.children()
.eq(2)
.eq(3)
.should('not.have.class', 'active')
.next()
.should('have.class', 'active')
Expand Down Expand Up @@ -451,16 +493,16 @@ const selectors = {
// only the variant items controled by the witnesses filter and referring to the target should be selected - 3rd and 4th variant item
cy.get(selectors.list)
.children()
.should('have.length', 6)
.should('have.length', 8)
.eq(0).should('contain','Ming. syr. 258').and('not.have.class','active')
.next().should('contain','Sach. 339').and('not.have.class','active') // eq(1)
.next() // separation line
.next().should('contain','Ming. syr. 258').and('have.class','active') // eq(2)
.next().should('contain','Sach. 339').and('have.class','active')
.next() // separation line
.next().should('contain','Ming. syr. 258').and('not.have.class','active')
.next().should('contain','Sach. 339').and('not.have.class','active')
})


})

describe('Single select mode', () => {
Expand Down Expand Up @@ -516,7 +558,7 @@ const selectors = {
.get(selectors.list)
.should('be.visible')
.children()
.should("have.length", 11) // we have 11 variant items as in the normal mode - no single select mode
.should("have.length", 13) // we have 11 variant items as in the normal mode - no single select mode
})
})
})
Expand Down
Loading