Skip to content

Conversation

@Anemy
Copy link
Member

@Anemy Anemy commented Oct 11, 2022

COMPASS-6186

Will remove the release notes label before merging as this isn't a released bug.
This PR fixes the menus not showing in the crud view. We're fixing it by removing the overflow: none in the KeylineCard component. I think this is better than changing the menu usePortal={true} for now. Currently when an element that a portal is referring to is removed from the dom, the menu teleports to the top left while it collapses. We could work around this, but I think usePortal={false} is a cleaner approach.
Did some drive by cleanup, removing old backgrounds.
Also added another 16px of margin to the bottom of the document list for a little better ux with the menus.

Because of these changes the previous box shadow on document cards being edited is now shown (it was hidden by the hidden overflow before). Do we want to remove this shadow? cc @mcasimir

Before After
Screen Shot 2022-10-11 at 2 34 39 PM Screen Shot 2022-10-11 at 2 35 16 PM

@mcasimir
Copy link
Collaborator

Ditching the overflow hidden and the background makes sense to me, good call! Let's also remove the shadow, if we want them back we can re-add them later together with design.

@Anemy Anemy requested a review from mcasimir October 12, 2022 15:31

.json-document-is-editing {
box-shadow: 2px 5px 8px fade(@palette__black, 20%);
border-radius: inherit;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice trick, didn't know that we could have done this!

@Anemy Anemy merged commit 27e900f into main Oct 13, 2022
@Anemy Anemy deleted the COMPASS-6186-fix-menus-in-crud branch October 13, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants