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

keeping sidebars and header visible on mobile view #949

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

keeping sidebars and header visible on mobile view #949

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
0a1eb6f
keeping sidebars and header visibile on mobile view
404mike Jan 5, 2024
125a86d
hide left sidebar if on mobile view
404mike Jan 5, 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
3 changes: 3 additions & 0 deletions src/content-handlers/iiif/modules/uv-shared-module/LeftPanel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export class LeftPanel<
this.options.panelOpen
);

const viewportWidth: number = document.documentElement.clientWidth;
if(viewportWidth <= 767) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the functionality that this provides, but I wonder if we can implement it in a more flexible way that avoids hard-coded magic numbers. Might it make sense to replace the panelOpen boolean configuration setting with a panelOpenMinWidth integer setting? Then it could be 0 to always open, some incredibly high number to always close, or a configurable breakpoint the rest of the time. What do you think? @edsilv?

Copy link
Contributor

Choose a reason for hiding this comment

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

@demiankatz One possibility that avoids any config stuff would be to assume that this behaviour is only needed when it's a portrait-oriented, touch-enabled device, which I think in 99% of cases would be a phone/tablet?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true, but I think @edsilv's point was that displaying the sidebar on mobile isn't the ideal solution to the problem, regardless of what approach we take for deciding to do it. Though if it's an urgent local need, I think the approach here might be able to be locally adopted as a workaround until the more complete solution is ready.


if (shouldOpenPanel) {
this.toggle(true);
}
Expand Down
6 changes: 3 additions & 3 deletions src/content-handlers/iiif/modules/uv-shared-module/css/xl.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
}

.headerPanel {
.only-desktop();
display: block !important;
}

.mainPanel {
.leftPanel {
.only-desktop();
display: block !important;
}

.rightPanel {
.only-desktop();
display: block !important;
}
}

Expand Down