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

profile: Fix glitch when exiting custom profile display UI. #9706

Closed

Conversation

shubhamdhama
Copy link
Member

Fixes: #9702.
GIF:
peek 2018-06-07 14-07

@zulipbot
Copy link
Member

zulipbot commented Jun 7, 2018

Hello @zulip/server-settings members, this pull request was labeled with the "area: settings (user)" label, so you may want to check it out!

@timabbott
Copy link
Sponsor Member

@shubhamdhama can you talk about how this works and why we should be confident this won't create problems with other modals? E.g. why is 50% correct?

@shubhamdhama
Copy link
Member Author

@timabbott
Three-point we should understand:

  • First this applies to hidden modals only because for visible .in containing modal this is overridden.
  • We have transform: translateY(-50%); so all the positioning will be done according to the center of modal.
  • When we close modal first it moves negative top of the screen and then it hides.

For most of modal top: -25% is more than the whole height of modal but here the modal is so large that the remaining part of modal is visible to us i.e. Height of modal > 25% screen. And this remaining causes glitch. But having set it to -50% is just equivalent to moving it to the same position in a screen of the same size but it negative top(kinda parallel universe :).
This is my understanding and visualization of the issue, hope this makes sense.

@shubhamdhama
Copy link
Member Author

shubhamdhama commented Jun 7, 2018

I should add this to commit message if this makes sense.
Oops.. late :)

@timabbott
Copy link
Sponsor Member

OK, that's what I was imagining. Merged as ed7373f, after adding a brief comment explaining that detail. Could also be good to have the longer explanation in the commit message. To provide a bit of reasoning here:

  • Anything that answers the question "why does this code do this strange thing" best belongsin a code comment, since that's what'll be most visible to folks reading the code later.
  • Anything that explains "why did we make this change" or "what research or reasoning was done to determine this code (change) is correct" generally belongs in the commit message. This is really valuable for folks trying to understand whether changing the code later will break something, but is much more buried than code comments (but also adds much less clutter).

Both are really valuable for saving reviewers time guessing why you did something :)

@timabbott timabbott closed this Jun 7, 2018
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.

None yet

3 participants