Skip to content

Use a ResizeObserver to update the layout of PDFFindBar #17152

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 1 commit into from
Oct 21, 2023

Conversation

Snuffleupagus
Copy link
Collaborator

Please note: In the Firefox PDF Viewer this findbar is only used for PDF documents placed in e.g. <iframe> elements.

By registering a ResizeObserver when the PDFFindBar is open we slightly unify and simplify how the findbar layout (row vs column) is handled.
This will be especially helpful with upcoming changes, where we'll make use of "data-l10n-id"/"data-l10n-args" to trigger translation in the viewer.

*Please note:* In the Firefox PDF Viewer this findbar is only used for PDF documents placed in e.g. `<iframe>` elements.

By registering a `ResizeObserver` when the `PDFFindBar` is open we slightly unify and simplify how the findbar layout (row vs column) is handled.
This will be especially helpful with upcoming changes, where we'll make use of "data-l10n-id"/"data-l10n-args" to trigger translation in the viewer.
@Snuffleupagus Snuffleupagus force-pushed the findbar-resizeObserver branch from 2e4f582 to e20ef39 Compare October 21, 2023 14:18
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/5d776faa3756ea9/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/5d776faa3756ea9/output.txt

Total script time: 1.44 mins

Published

@calixteman
Copy link
Contributor

Can't we achieve the same layout change with flex-wrap ?

@Snuffleupagus
Copy link
Collaborator Author

Can't we achieve the same layout change with flex-wrap ?

Unfortunately I don't believe that this helps, since the findbar then becomes too wide once wrapping occurs; please note

pdf.js/web/pdf_find_bar.js

Lines 195 to 198 in a4cd2ef

// The find bar has an absolute position and thus the browser extends
// its width to the maximum possible width once the find bar does not fit
// entirely within the window anymore (and its elements are automatically
// wrapped). Here we detect and fix that.

@calixteman
Copy link
Contributor

Ok, that's unfortunate, that said I way prefer this solution (i.e. the one in this patch).

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@Snuffleupagus Snuffleupagus removed the request for review from timvandermeij October 21, 2023 15:44
@Snuffleupagus Snuffleupagus merged commit 4c4676e into mozilla:master Oct 21, 2023
@Snuffleupagus Snuffleupagus deleted the findbar-resizeObserver branch October 21, 2023 15:46
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.

3 participants