Skip to content

Conversation

Saira-A
Copy link
Contributor

@Saira-A Saira-A commented Mar 10, 2025

This adds the help button from #1322 to the mobile footer panel. Switched on in config for testing - will set to false before merging.

Copy link

vercel bot commented Mar 10, 2025

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 12, 2025 1:07pm

Copy link
Contributor

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @Saira-A -- this looks good to me, but I think as we discussed on today's standup, there may be some question about whether this belongs specifically in the MobileFooterPanel or if it can be incorporated into the generic FooterPanel to make it more broadly available. See @jamesmisson's work in #1330 for a point of comparison. (We'll also need to figure out if this has any implications in combination with #1330 due to the increasing number of buttons falling into the footer).

I'm "requesting changes" to ensure that we don't merge this prematurely, but I don't necessarily think changes need to be made right away. Maybe we should discuss this further during or after tomorrow's standup, and maybe the path forward will become more clear once all of the other outstanding work on panels and buttons gets merged together over the next day or so.

@LanieOkorodudu
Copy link
Collaborator

Thanks, @Saira-A -- this looks good to me, but I think as we discussed on today's standup, there may be some question about whether this belongs specifically in the MobileFooterPanel or if it can be incorporated into the generic FooterPanel to make it more broadly available. See @jamesmisson's work in #1330 for a point of comparison. (We'll also need to figure out if this has any implications in combination with #1330 due to the increasing number of buttons falling into the footer).

I'm "requesting changes" to ensure that we don't merge this prematurely, but I don't necessarily think changes need to be made right away. Maybe we should discuss this further during or after tomorrow's standup, and maybe the path forward will become more clear once all of the other outstanding work on panels and buttons gets merged together over the next day or so.

Thanks so much for this work, @Saira-A! I agree with @demiankatz that the MobileFooterPanel is starting to feel a bit crowded. Just a small suggestion, I wonder if the Help button might look better positioned between the More Info and Download buttons? Totally open to discussion, but thought I'd share in case it's helpful.

@demiankatz
Copy link
Contributor

@LanieOkorodudu, also note when discussing button positions that #1330 proposes moving the "More Info" button all the way to the right side, to more clearly associate it with opening/closing the right panel. (The newly-introduced button for toggling the left panel will be all the way on the left). This change might influence the positioning of the new help button as well.

Copy link
Contributor

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @Saira-A!

@demiankatz demiankatz merged commit d62ed56 into UniversalViewer:dev Mar 13, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from IN TESTING to COMPLETED in Community Sprint Feb 2025 Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: COMPLETED

Development

Successfully merging this pull request may close these issues.

3 participants