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: UI bug for user message to take available screen space #32

Conversation

jasbindar-singh
Copy link

@jasbindar-singh jasbindar-singh commented Oct 11, 2020

PR for issue number #31

  • Add a function to check available screen space left for the user message
  • Set the height of the user-message wrapper parent height to the available space
  • Set the max-height for the user-message to be 100% of parents height(So, if available user message height > available space then show scroll else take up the available space)

@TheShubham99
Copy link
Member

Thanks for the PR.
I'll review it asap.

Few suggestions-

  1. Follow the commit conventions.
    (You can have a look at previous commits)

I guess this is a UI bug fix so tag it as a fix:

  1. Mention the issue in the PR description.

@jasbindar-singh jasbindar-singh force-pushed the feature/add-available-space-logic branch from ff5f107 to 13ef97a Compare October 12, 2020 05:32
@jasbindar-singh jasbindar-singh changed the title --added a function to calculate available space and set scroll after … fix: UI bug for user message to take available screen space Oct 12, 2020
@jasbindar-singh
Copy link
Author

Made the suggested edits.

Copy link
Member

@TheShubham99 TheShubham99 left a comment

Choose a reason for hiding this comment

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

@jasbindar-singh
Good yet custom approach 👍

I am not sure if it will work on all types of screens. (especially the bigger screens)

image

ping @pollend :)

@jasbindar-singh
Copy link
Author

@TheShubham99 Np, let me know if you have some suggestion, and what screen size did you test it on? And by bigger screen you emphasize on width or height?

@TheShubham99 TheShubham99 requested a review from pollend October 12, 2020 17:03
@pollend
Copy link
Member

pollend commented Oct 13, 2020

Just kind of wondering why this needs to be done through javascript. seems like the top margin is just some percentage of the screen height. I think vh units are relative to a percentage of the viewport height. Might be a better way to accomplish the same affect without custom javascript.

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.

3 participants