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

update CSS to prevent event title wrapping #2315

Merged
merged 3 commits into from Oct 23, 2019
Merged

Conversation

@evlnyng
Copy link
Contributor

evlnyng commented Oct 21, 2019

Description

Fixes #2299.

  • Increased the bottom-margin for event icons to prevent long titles from wrapping beneath it.

Further comments

This fix does increase the space between the events (pictured below; original on the left and my version on the right).

Screen Shot 2019-10-20 at 12 24 44 AM

If this is not preferred, please let me know and I will try a different solution.

Checklist

This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have added necessary documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • Any dependent changes have been merged and published in downstream modules (if applicable)

@nasa-gibs/worldview

@jasontk19 jasontk19 self-requested a review Oct 22, 2019
@jasontk19 jasontk19 changed the base branch from master to develop Oct 22, 2019
@jasontk19

This comment has been minimized.

Copy link
Contributor

jasontk19 commented Oct 22, 2019

Nice! I think this would work, but I bet we can prevent the wrap without adding additional vertical margin if we give the h4 element some left margin to account for the icon:
Screen Shot 2019-10-22 at 9 20 33 AM
Screen Shot 2019-10-22 at 9 19 48 AM

Also, it looks like you're working off of your own forked version of our repo. If you can, go ahead and clone our repo directly and create your branch there. This way other developers won't have to clone your fork to checkout your branches to test them. Let me know if that doesn't make sense or you want some help with it!

@evlnyng

This comment has been minimized.

Copy link
Contributor Author

evlnyng commented Oct 23, 2019

Understood! I can absolutely do that, thank you. I will submit a new request with the new branch.

@evlnyng evlnyng closed this Oct 23, 2019
@jasontk19

This comment has been minimized.

Copy link
Contributor

jasontk19 commented Oct 23, 2019

@evlnyng I was mistaken; it turns out you need to have permissions to push a branch to this repository directly. You can open the PR off of your forked branch like you did originally!

also reverted changes to icon margins
@evlnyng evlnyng reopened this Oct 23, 2019
@evlnyng

This comment has been minimized.

Copy link
Contributor Author

evlnyng commented Oct 23, 2019

I thought that might've been the case! I have reverted the changes to the event icon and added the left margin to the h4 element. Thank you for your patience!

Update margin-left syntax
@@ -24,6 +24,7 @@
font-size: 0.85em;
color: #ccc;
padding: 0 3px;
left-margin: 40px;

This comment has been minimized.

Copy link
@jasontk19

jasontk19 Oct 23, 2019

Contributor

Almost there! The proper syntax here is going to be margin-left

This comment has been minimized.

Copy link
@evlnyng

evlnyng Oct 23, 2019

Author Contributor

How embarassing!. 🤦‍♀️ Serves me right for trying to rush it during a break at work.

This comment has been minimized.

Copy link
@jasontk19

jasontk19 Oct 23, 2019

Contributor

No worries! I think we all do the same sort of thing pretty much daily 👍

Thanks for your contribution!

@jasontk19 jasontk19 merged commit 9d65120 into nasa-gibs:develop Oct 23, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@ZachTRice ZachTRice mentioned this pull request Nov 21, 2019
@ZachTRice ZachTRice mentioned this pull request Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.