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

Move autolock timer #3743

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

radchukd
Copy link

Resolves #3572

What

Move auto-lock timer
autolock

Fix SharedSelect trigger z-index
settings

Fix OnboardingDerivationPathSelect width
width

Fix SharedSelect trigger z-index
Fix OnboardingDerivationPathSelect width
Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Not quite ready to merge here as I'm worried that the z-index will have unintended side-effects. In particular, this z-index was introduced in 8cf22e4, which handled the slippage tolerance dropdown for swaps—wondering if removing it creates problems there.

@@ -167,7 +167,6 @@ export default function SharedSelect(props: Props): ReactElement {

.button {
position: relative;
z-index: 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Worried that this misses a place where the z-index does matter. Are we sure it doesn't?

Copy link
Author

Choose a reason for hiding this comment

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

is being used in 2 places as of now:

  • <Settings />, where it actually overflows another one, hence the removal of z-index (screenshot 2)
  • <OnboardingDerivationPathSelect />, where z-index does not affect anything.

IMO, this select trigger is a button and there is no reason for it to have an abnormal z-index, especially if it's higher than select menu.

@@ -180,6 +180,7 @@ export default function ImportSeed(props: Props): ReactElement {
margin-top: 16px;
}
.select_wrapper {
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

@@ -412,6 +412,7 @@ export default function Settings(): ReactElement {
FeatureFlags.SUPPORT_ACHIEVEMENTS_BANNER,
notificationBanner,
),
autoLockSettings,
Copy link
Contributor

Choose a reason for hiding this comment

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

Solid catch!

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.

Settings: Move auto lock timer
2 participants