Skip to content

Commit

Permalink
fix: popover with very narrow text (#1894)
Browse files Browse the repository at this point in the history
#1702

If the page is resized before any footnotes have been opened, the
`resize` action can compute a very small `max-width` based on zero size
footnote contents. This was allowed to happen because `isMounted` always
returns true after the popover element is created but not yet shown.

- `isMounted` now checks whether the popover is contained in the DOM.
- `createElementFromHTML` now unwraps newly created elements to improve
  robustness in case `parentElement` gets used elsewhere.
  • Loading branch information
kevin1 committed May 16, 2024
1 parent 31c9f37 commit 5c7633e
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
11 changes: 10 additions & 1 deletion cypress/e2e/click.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const DEFAULT_VIEWPORT_HEIGHT = 600

context('Click', () => {
beforeEach(() => {
cy.viewport(800, 600)
cy.viewport(800, DEFAULT_VIEWPORT_HEIGHT)
cy.visit('/cypress/e2e/fixtures/click.html')
})

Expand All @@ -17,4 +19,11 @@ context('Click', () => {
cy.get('.littlefoot__popover').should('not.have.class', 'is-scrollable')
cy.get('.littlefoot__content').should('not.have.attr', 'tabindex')
})

it('popover layout upon resizing window before activation (#1702)', () => {
// Resize the viewport before any footnotes have been activated.
cy.viewport(800, DEFAULT_VIEWPORT_HEIGHT + 100)
cy.findByTitle('See Footnote 1').click()
cy.get('.littlefoot__content').invoke('width').should('be.greaterThan', 100)
})
})
5 changes: 4 additions & 1 deletion src/dom/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ function getByClassName<E extends Element>(element: E, className: string): E {
function createElementFromHTML(html: string): HTMLElement {
const container = document.createElement('div')
container.innerHTML = html
return container.firstElementChild as HTMLElement
const element = container.firstElementChild as HTMLElement
// Remove element from container div.
element.remove()
return element
}

function children(element: Element, selector: string): readonly Element[] {
Expand Down
2 changes: 1 addition & 1 deletion src/dom/footnote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export type FootnoteElements = Readonly<{
wrapper: HTMLElement
}>

const isMounted = (popover: HTMLElement) => !!popover.parentElement
const isMounted = (popover: HTMLElement) => document.body.contains(popover)

export function footnoteActions({
id,
Expand Down

0 comments on commit 5c7633e

Please sign in to comment.