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

Adding space in between the skill bubble on the skill page in mobile view #3553

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

Conversation

1209simran
Copy link
Member

Fixes #3552

Changes: Added space in between the skill bubbles and width of the skill bubble section changed to 100%.

Demo Link: https://pr-3552-fossasia-susi-web-chat.surge.sh

Screenshots:
Before
WhatsApp Image 2020-05-22 at 22 17 45

After
WhatsApp Image 2020-05-22 at 23 29 10

Copy link
Member

@akshatnitd akshatnitd left a comment

Choose a reason for hiding this comment

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

Please check the desktop view as well.

@1209simran
Copy link
Member Author

1209simran commented May 24, 2020

@akshatnitd , i've made the changes in desktop view too. Changed the width from 55% to 100%.
It looks more cleaner than before.
DeepinScreenshot_20200524140227

@@ -359,7 +359,7 @@ class SkillListing extends Component {
key={index}
data={data}
history={history}
margin={'1.5% 2.85% 1.5% 0'}
margin={'12px 10px 12px 0'}
Copy link
Member

Choose a reason for hiding this comment

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

Why pixles insted of percentage ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Percentage is a relative unit. So, the margin will change according to the screen of size.On a really small screen like a phone that might be appropriate. But if you're viewing that same page on a bigger size screen, that margin is going to be huge if we use percentage. That's why I've used pixels to maintain the same gap between them.

Copy link
Member

@plxity plxity left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@1209simran
Copy link
Member Author

@akshatnitd , please suggest if there are some more changes.

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.

Adding space in between the skill bubble on the skill page in mobile view
5 participants