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

fix(suite): Adjust IO dropdowns style for Coinjoin transaction detail #13146

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

hstastna
Copy link
Contributor

@hstastna hstastna commented Jul 2, 2024

Description

Drop-down selector size in Coinjoin transaction detail window was unexpectedly changing after opening or while hovering. After removing unnecessary css of related StyledCollapsibleBox, adding proper and missing Divider component instead of achieving the same using css and after adjusting related padding values, we achieve the expected look.

Related Issue

Resolve #13098

Videos:

Before:

coin_before.mp4

After:

coin_after.mp4

@hstastna hstastna force-pushed the fix/13098-dropdown-transaction-size-changes branch from 7177b31 to 0f3a631 Compare July 2, 2024 15:49
@hstastna hstastna force-pushed the fix/13098-dropdown-transaction-size-changes branch 2 times, most recently from 5ef9ed5 to 7d9c507 Compare July 8, 2024 08:23
@@ -34,38 +27,15 @@ const Wrapper = styled.div`
`;

const StyledCollapsibleBox = styled(CollapsibleBox)<{ $elevation: Elevation }>`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use it without StyledCollapsibleBox, just using CollapsibleBox?

Also I think you don't need $elevation here and I think also other props are not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need elevation here, right. But adjusting CollapsibleBox.Header and CollapsibleBox.Content is needed to achieve a nice and consistent look in different window sizes and zoom levels. Not sure I can adjust those sub-components the other way if we don't export them too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've asked the designer on his opinion about CollapsibleBox look in the various places, will update the PR accordingly.

Copy link
Contributor Author

@hstastna hstastna Jul 9, 2024

Choose a reason for hiding this comment

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

Update: From what I've found from the designer, we want the existing padding for CollapsibleBox there, so as I've implemented, and there is some update already prepared. I'd suggest to bring newer CollapsibleBox style in followup PRs, not to mix it here. This PR's aim was just to fix what got broken to display the related drop down for coinjoin transaction the nicer way.

@hstastna hstastna force-pushed the fix/13098-dropdown-transaction-size-changes branch 3 times, most recently from badbdd0 to 9ae98c0 Compare July 9, 2024 13:59
@hstastna hstastna requested a review from jvaclavik July 9, 2024 13:59
@hstastna hstastna force-pushed the fix/13098-dropdown-transaction-size-changes branch from 9ae98c0 to b51b62f Compare July 10, 2024 08:16
@hstastna hstastna force-pushed the fix/13098-dropdown-transaction-size-changes branch from b51b62f to 8be6a31 Compare July 10, 2024 08:17
@hstastna hstastna merged commit fa3e98e into develop Jul 10, 2024
15 of 24 checks passed
@hstastna hstastna deleted the fix/13098-dropdown-transaction-size-changes branch July 10, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop-down selector size changes after opening.
2 participants