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.

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

ENG-4066 fix(portal): create claim combobox search fix #852

Conversation

Vitalsine85
Copy link
Member

Affected Packages

Apps

  • portal

Packages

  • 1ui
  • api
  • protocol
  • sdk

Tools

  • tools

Overview

This adds a check if the user is accessing the app from mobile and changes the modal prop on the Popover conditionally. Setting modal to true can cause some weird behavior on mobile. I was only able to test this locally in the dev console, so we will want to double check that it works once this is on staging.

Screen Captures

If applicable, add screenshots or screen captures of your changes.

Declaration

  • I hereby declare that I have abided by the rules and regulations as outlined in the CONTRIBUTING.md

Copy link

linear bot commented Sep 26, 2024

ENG-4066 [Bug] Can’t create Claims on mobile due to formatting issues

When trying to select a Predicate or an Object, the search bar opens upwards like depicted in the screenshot attached, preventing you from searching for the Identity you want to compose your claim with.

IMG_2574.png

@github-actions github-actions bot added the fix Fix label Sep 26, 2024
@Vitalsine85 Vitalsine85 enabled auto-merge (squash) September 26, 2024 19:39
return (
<Popover
open={isObjectPopoverOpen}
onOpenChange={setIsObjectPopoverOpen}
modal={isObjectPopoverOpen}
modal={isMobileView ? false : isObjectPopoverOpen}
Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting -- my understanding was that modal prop in Radix's Popover changed how it interacts with background objects

Is this also how they recommend solving the unexpected collision issues we had with the direction it opens?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, apparently when modal is set to true it disregards the placement and other properties. Can't really fully test this until it is on dev since I can't access localhost from my mobile device.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I remember correctly we had to set it to true so that scrolling worked on desktop. Everything still functions properly on desktop with this. I think that is going to be the main thing to test, scrolling on mobile with this change.

Copy link
Member

Choose a reason for hiding this comment

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

Got it -- approved! Let's keep an eye on that in QA

@Vitalsine85 Vitalsine85 merged commit 7dafe42 into main Oct 2, 2024
3 checks passed
@Vitalsine85 Vitalsine85 deleted the vital/eng-4066-bug-cant-create-claims-on-mobile-due-to-formatting-issues branch October 2, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants