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

Add additional css margin controls to Feedback Modal #14539

Open
craigbehnke opened this issue Nov 30, 2024 · 1 comment
Open

Add additional css margin controls to Feedback Modal #14539

craigbehnke opened this issue Nov 30, 2024 · 1 comment

Comments

@craigbehnke
Copy link

craigbehnke commented Nov 30, 2024

Problem Statement

Add additional CSS variables to specify specific margin amounts for top/bottom/left/right on the feedback modal.

In order to fit the modal into a screen on platforms like Ionic Capacitor on mobile, it would be incredibly valuable to have additional css variables to add a margin around the div.dialog__position beyond the inset.

This is the current behavior without any CSS variables applied, purely out of the box:
Image

This is what I want to be able to do using the individual margin variables:
Image

This is what I can do with the current tooling, setting only --page-margin:
Image

Solution Brainstorm

Add additional CSS variables for configuration:

  • --page-margin-top
  • --page-margin-bottom
  • --page-margin-left
  • --page-margin-right

These variable names are consistent with the existing --page-margin variable.

These variables could then be included in the .dialog__position class to specify the configured margin, while also defaulting to existing behavior:

.dialog__position {
    /* Existing properties in the class are here */
    margin-top: calc(var(--page-margin-top,--page-margin) - var(--page-margin));
    margin-bottom: calc(var(--page-margin-bottom,--page-margin) - var(--page-margin));
    margin-left: calc(var(--page-margin-left,--page-margin) - var(--page-margin));
    margin-right: calc(var(--page-margin-right,--page-margin) - var(--page-margin));
}

In a case where the developer does not set the specific variables, each statement would evaluate to --page-margin - --page-margin, which would always be 0px.

In a case where the developer has configured the specific margin variable, however, it would add the difference between the desired margin and the default page margin, effectively overriding it.

As a note, it appears that this functionally only applies in cases where the window is 600px wide or less (i.e. mobile) where the mode switches from padding to inset.

Edit: Current Workaround

After looking at the code a bit more, I did find a workaround that works but is incredibly hack-y: I can set the --page-margin variable with the arguments for the inset property, like this:

@media (max-width: 600px) {
    div#sentry-feedback{
        --page-margin: var(--ion-safe-area-top, 1rem)
            var(--ion-safe-area-right, 1rem) var(--ion-safe-area-bottom, 1rem)
            var(--ion-safe-area-left, 1rem);
    }
}

The reason why this is incredibly hack-y is that the docs specify the default value as 16px, which means that this is not CSS typesafe.

@s1gr1d s1gr1d transferred this issue from getsentry/sentry-javascript Dec 2, 2024
@s1gr1d s1gr1d transferred this issue from getsentry/sentry-capacitor Dec 2, 2024
@s1gr1d
Copy link
Member

s1gr1d commented Dec 2, 2024

Hy, your workaround seems fine as you can define all margins with the shorthand margin. If only one value is provided, this value is used on all sides. It's totally normal to provide four values.

Or maybe I understand something wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Community
Status: Needs Discussion
Development

No branches or pull requests

3 participants