twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
GNU Affero General Public License v3.0
15.79k stars 1.73k forks source link

[bug 🐛 ] : fix page header accessible when deleting webhook #6817

Open harshit078 opened 2 weeks ago

harshit078 commented 2 weeks ago

Bug Description

Current Behaviour

https://github.com/user-attachments/assets/75b18ffd-0f65-481c-9065-7cd09a315ba3

Expected behavior

https://github.com/user-attachments/assets/cbac0b56-578f-49d5-a092-84f936016ca6

Technical inputs

Example:

- Change propties of Header and apply CSS properties
- Rearrange the logic for Save and Cancel button for mobile viewport(768)
harshit078 commented 2 weeks ago

Proposal

Screenshot 2024-08-31 at 7 14 03 PM
charlesBochet commented 1 week ago

@harshit078 I'm not sure to see the issue When you hit 'Delete' the modal background overlay should make impossible to click on the button (and if they are keyboard listener they should be disabled too)

Isn't it the case?

charlesBochet commented 1 week ago

I see that we have the following behavior:

image

I don't think this is right and will likely be fixed in an upcoming PR

harshit078 commented 1 week ago

I already came up with the solution and tested it locally have it ready with me. If not taken internally, can I push the PR on it ? @charlesBochet

Bonapara commented 1 week ago

We should revert to that behavior indeed:

When you hit 'Delete' the modal background overlay should make impossible to click on the button (and if they are keyboard listener they should be disabled too)

I think there is another PR on this one: https://github.com/twentyhq/twenty/issues/6737#issuecomment-2323315465

harshit078 commented 1 week ago

Would eliminating - z-index:20; on PageHeader.tsx

and adding -

  @media (max-width: ${MOBILE_VIEWPORT}px) {
    width: 93%;
    padding-top: ${({ theme }) => theme.spacing(3)};
    }

be a correct apporach ? As in the video attached, it shows the exact code running.

harshit078 commented 4 days ago

@charlesBochet @Bonapara , is this issue resolved ?

Bonapara commented 4 days ago

Just checked—nope, it's still there!

harshit078 commented 4 days ago

@Bonapara, Can I work it on then ?

Bonapara commented 3 days ago

Sure @harshit078, thanks for contributing!