Skip to content

Preliminary sidebar handling css #1320

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

Jump to bottom
Merged
merged 15 commits into from
Mar 3, 2025
Merged

Preliminary sidebar handling css #1320

merged 15 commits into from
Mar 3, 2025

Conversation

LlGC-jop
Copy link
Contributor

Updates CSS and some small JS bits to allow the side panels to appear on mobile.

Currently only tested on OSD extension, but should work the same for all types.

Please excuse the garish colours - they're useful for checking which media queries are running and the stacking of the z-indexes :)

Summary of changes:

  • CSS to hide the left and right panels (L & R from now on) has been removed, building on James' changes.
  • As the panels are now 'visible' all the time, they're pulled off the viewport with a transform property. Aria props set by JS should still apply correctly.
  • The main panel has overflow hidden to allow this method of hiding. For now they still appear on the page just so they're clickable for testing, although in future there'll be header buttons to achieve this.
  • At mobile sizes the L & R are absolutely positioned so that they'll 'float' over the top of the Main panel
  • At medium and up the main panel has a grid layout with three columns so there's no need for JS to calculate and set widths for things
  • CSS custom properties are used for the grid areas widths and to toggle animations. These will eventually need to be set as an inline style on the <html> element, which will allow config from JS to be passed to the CSS, but for now they're all in styles.less in the :root selector to simulate this.
  • Animations are handled with CSS transitions. The --uv-animation property acts as a boolean toggle for this and can be set to 0 to disable animations without the need for more complex selectors.

jamesmisson and others added 13 commits February 26, 2025 17:08
@jamesmisson
fix css media query mechanism
34ad02f
@jamesmisson
fix whitespace
6a886a6
@LlGC-jop
Initial grid
a734057
@LlGC-jop
Merge remote-tracking branch 'JamesMisson/UX-mobile-moreinfo' into dev
b2010e0
@LlGC-jop
Add toggle function to the specific panels so classes can be set
8ec68e2
@LlGC-jop
Add open class and change animation handling to let css do it, with t…
aada409 
…imeout to account for transition time
@LlGC-jop
Comment out hiding of side panels on mobile, as they now need to be d…
2c5a1d0 
…isplayed all the time, with other css handling visibility
@LlGC-jop
More commenting to keep panels visible all the time
2b912ba
@LlGC-jop
Preliminary css to handling positioning and display of side panels
e71f90d
@github-actions
Commit from GitHub Actions (Lint code)
2add343
@LlGC-jop
Merge branch 'UniversalViewer:dev' into dev
763e374
@LlGC-jop
Move props to the root element to simulate setting html tag
94a8e0a
@LlGC-jop
Merge branch 'dev' of https://github.com/LlGC-jop/universalviewer int…
941bf47 
…o dev
@vercel Vercel
Copy link

vercel bot commented Feb 28, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
universalviewer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 3, 2025 1:57pm

@LlGC-jop LlGC-jop marked this pull request as draft February 28, 2025 10:15
@LlGC-jop
Copy link
Contributor Author

Still to do is the gallery view for OSD.

I'm also going to remove some of the show()/hide() jQuery calls so that visiblity of the panels' top/close/main child elements are controlled purely in the CSS if possible.

@jamesmisson
Copy link
Contributor

Looks good on desktop and mobile for me, thanks Johnathan. I'll merge this into my branch and apply the button event.

Using this made me think the thumbnail panel should close automatically when a thumbnail is selected on mobile. Unless anyone thinks otherwise I'll look into that.

@demiankatz
Copy link
Contributor

Using this made me think the thumbnail panel should close automatically when a thumbnail is selected on mobile. Unless anyone thinks otherwise I'll look into that.

Yes, that makes sense to me.

@jamesmisson jamesmisson mentioned this pull request Feb 28, 2025
Closed
@demiankatz demiankatz moved this to WORK IN PROGRESS in Community Sprint Feb 2025 Mar 3, 2025
crhallberg
crhallberg reviewed Mar 3, 2025
View reviewed changes
Copy link
Contributor

@crhallberg crhallberg left a comment

Choose a reason for hiding this comment

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

This is definitely on the right track with the approach and configuration considerations. For my money, we should keep the new color scheme.

@crhallberg
Copy link
Contributor

Do we want to standardize UV css variables to start with --uv-? I can align #1314 with this.

@LlGC-jop
Copy link
Contributor Author

LlGC-jop commented Mar 3, 2025 •
edited
Loading

Do we want to standardize UV css variables to start with --uv-? I can align #1314 with this.

@crhallberg I was just basing it on the --uv-vh property that's added to <html> on the dev page, but I think 'namespacing' our properties would be best.

But I really don't want to type --universalviewer, so --uv gets my vote😄

@LlGC-jop LlGC-jop merged commit 5973356 into UniversalViewer:dev Mar 3, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from WORK IN PROGRESS to COMPLETED in Community Sprint Feb 2025 Mar 3, 2025
@demiankatz
Copy link
Contributor

@LlGC-jop, did you intend to merge this to dev? It hasn't been approved yet, so I'm not sure how that was able to happen...

@crhallberg crhallberg mentioned this pull request Mar 3, 2025
Open
3 tasks
@jamesmisson
Copy link
Contributor

Hi @LlGC-jop are there more CSS changes to come or can I pull this down to my branch?

@demiankatz
Copy link
Contributor

@jamesmisson, because this branch got accidentally merged and then reverted, it probably won't merge cleanly with your own branch. I think @LlGC-jop has set up a new branch to replace this; I'll let him share the details since I'm not sure if I'm entirely correct about this. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crhallberg crhallberg crhallberg left review comments

@demiankatz demiankatz Awaiting requested review from demiankatz

@jamesmisson jamesmisson Awaiting requested review from jamesmisson

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Community Sprint Feb 2025
Status: COMPLETED
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LlGC-jop @jamesmisson @demiankatz @crhallberg