Skip to content

PB-73: Several UI improvement in import tool #598

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

Merged
merged 10 commits into from
Jan 12, 2024

Conversation

ltshb
Copy link
Contributor

@ltshb ltshb commented Jan 9, 2024

  • Placeholder translation updated
  • removed the search bar for the geo catalogue, the regular search bar in the header should be use.
  • Added color background on text search emphasize
  • Added tooltip for truncated text in catalogue
  • Truncate and added tooltip for long layer title in active layers

Test link

Copy link

cypress bot commented Jan 9, 2024

Passing run #212 ↗︎

0 161 19 0 Flakiness 0

Details:

Skipped flaky test
Project: web-mapviewer Commit: 9f1d0a272e
Status: Passed Duration: 04:22 💡
Started: Jan 12, 2024 1:59 PM Ended: Jan 12, 2024 2:03 PM

Review all test suite changes for PR #598 ↗︎

@ltshb ltshb force-pushed the feat-PB-73-search-geo-catalogue-2 branch 3 times, most recently from ed294c0 to 8f65766 Compare January 10, 2024 12:39
@ltshb ltshb requested a review from pakb January 10, 2024 13:16
Comment on lines 63 to 68
cy.viewport(50, 480)
// Here unfortunately I could not find a way to correctly wait that the
// tippy correct got registered after the resize, therefore I've added a wait.
// I've tried spying on the initializeTippy method, but somehow the spy was never called.
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(1000)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pakb I'm not sure if I want to keep this test, I don't like the 1s wait. Unfortunately I did not find a way to make this test reliable. Without this wait is fails time to time.
I tried to spy on ResizeObserver and/or initializeTippy but without success the spy was never called. Maybe you have some better idea ?

Copy link
Contributor Author

@ltshb ltshb Jan 10, 2024

Choose a reason for hiding this comment

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

Here are some code that I tried

// Assertions based on the spy

const slotContent = 'My slot content'
// const { wrapper, component } = cy.mount(TextTruncate, {
//     slots: { default: slotContent },
// })
cy.mount(TextTruncate, {
    slots: { default: slotContent },
}).as('mount')
cy.get('@mount').then((wrapper) => {
    cy.spy(wrapper.component, 'initializeTippy').as('initializeTippy')

    cy.get('@initializeTippy').should('not.have.been.calledOnce')
    cy.get('[data-cy="inner-element"]').realHover({ position: 'left' })
    cy.get('[data-cy="outter-element"]').should('not.have.attr', 'aria-describedby')
    cy.get('.tippy-box').should('not.exist')
    cy.get('[data-cy="inner-element"]').click('left')

    cy.viewport(50, 480)
    cy.get('@resizeObserverSpy').should('have.been.calledOnce')
    cy.get('@initializeTippy').should('have.been.calledOnce')
    cy.get('[data-cy="inner-element"]').realHover({ position: 'left' })
    cy.get('[data-cy="outter-element"]').should('have.attr', 'aria-describedby')
    cy.get('.tippy-box').should('be.visible').contains(slotContent)
    cy.get('[data-cy="inner-element"]').click('left')

    cy.viewport(300, 480)
    cy.get('@initializeTippy').should('have.been.calledOnce')
    cy.get('[data-cy="inner-element"]').realHover({ position: 'left' })
    cy.get('[data-cy="outter-element"]').should('not.have.attr', 'aria-describedby')
    cy.get('.tippy-box').should('not.exist')
    cy.get('[data-cy="inner-element"]').click('left')
})

// Clean-up: restore the original ResizeObserver
win.ResizeObserver = originalResizeObserver

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

as discussed, try this wait/intercept approach to replace the arbitrary 1000ms wait, and we are good to go

@ltshb ltshb force-pushed the feat-PB-73-search-geo-catalogue-2 branch 2 times, most recently from 31eddb7 to 147060b Compare January 11, 2024 19:40
ltshb added 9 commits January 11, 2024 20:46
…in import tool

To search for layers from the geoadmin catalogue we should use the main bar.

The wms size is for no use for the user in the application therefore remove it.
Do not add the legend icon if no legend is really available and do not
add the description as well if only the legend is available.
In order to have the styling working we need to import it in the cypress support
file.

To do this the initial styling has been moved into a scss file in order to ease
the cypress setup.
@ltshb ltshb force-pushed the feat-PB-73-search-geo-catalogue-2 branch 4 times, most recently from 2b8e976 to f7e8763 Compare January 12, 2024 07:44
@ltshb
Copy link
Contributor Author

ltshb commented Jan 12, 2024

Also the tests now seems to be quite stable on the CI, it is not the case locally using the cypress gui and headless ! I'll try another approach using events instead of fake url.

@ltshb ltshb force-pushed the feat-PB-73-search-geo-catalogue-2 branch from f7e8763 to 9f1d0a2 Compare January 12, 2024 13:54
@ltshb
Copy link
Contributor Author

ltshb commented Jan 12, 2024

I could not find anyway to make the tests non flaky, therefore I skipped the one that causing issues for the moment

@ltshb ltshb merged commit 09e32e5 into develop Jan 12, 2024
@ltshb ltshb deleted the feat-PB-73-search-geo-catalogue-2 branch January 12, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants