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 inconsistent alignment in Language button and Fix Story description overlapping footer contents, if description text is large #193

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

prajwalkulkarni
Copy link

Issue Number

fixes #173
fixes #178

Describe the changes you've made

#173
Removed classes on <EOS_KEYBOARD_ARROW_UP />, <EOS_KEYBOARD_ARROW_DOWN /> components, along with the class on div that holds the string Language so that inconsistent margin/padding is eliminated.

#178
Replaced absolute viewport height of story page to 100% to contain the expanded story description and avoid overlapping and overflowing, and also setting the height to 100% ensures it occupies the full screen giving it the same behavior as 100vh.

Additionally, I've also reordered the properties of .options-bar class of home.scss prior to which the tests were failing due to incorrect lexical ordering of the class properties.
Describe if there is any unusual behavior (Any Warning) of your code(Write NA if there isn't)

NA
Additional context (OPTIONAL)

Test plan (OPTIONAL)

A good test plan should give instructions that someone else can easily follow.

Manual testing:

  1. Head to the login screen, observe the language button and verify if there's a proper alignment between the arrow icon and the string.
  2. Login and browse through different stories. If any story contains a longer description and is truncated, click on Show more and scroll all the way to the bottom. You should notice that the footer content is not overlapping any of the story components.
    Checklist
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • The title of my pull request is a short description of the requested changes.

Provide a Deployed link of route/page that needs to review

Preview: Deploy preview link here with the appropriate route

@@ -25,7 +25,7 @@
}

&-page {
height: 100vh;
height: 100%;
Copy link
Member

Choose a reason for hiding this comment

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

100% height is only taking the 100% for this element in particular. 100vh takes the 100% from the viewport. That's the main difference here. We need the 100vh or a massive workaround that wouldn't make sense to be honest.

image

Copy link
Member

Choose a reason for hiding this comment

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

I've tested this myself, and I can give you the solution to what you're looking for:

use: min-height: calc(100vh - 150px); and remove height

@@ -25,7 +25,7 @@
}

&-page {
height: 100vh;
height: 100%;
Copy link
Member

Choose a reason for hiding this comment

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

I've tested this myself, and I can give you the solution to what you're looking for:

use: min-height: calc(100vh - 150px); and remove height

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants