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 style regressions #697

Merged
merged 1 commit into from
Jan 18, 2018

Conversation

ZachTRice
Copy link
Contributor

Fixes #670, Fixes #671

Changes in this pull request:

  • Add "Segoe UI" default font back to "Add Layers" button
  • Change tour step 1 to be a wide window
  • Add bordered styles to tour windows which were removed from our version of joyride;
    this requires using an after element on the nub element whereas before there was a second element.
  • Fix margins in tour windows
  • Remove minimal-ui viewport attribute

Copy link
Contributor

@localjo localjo left a comment

Choose a reason for hiding this comment

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

Styles look good now. Just a few comments.

@@ -1,6 +1,10 @@
/*
* Default jquery-ui button
*/
.ui-button-text {
font-family: "Segoe UI", "Arial", sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the original "Segoe UI" declaration is still there, it's just overridden by these two lines added in 8345cb1 that we could probably remove to be more efficient.

web/css/tour.css Outdated
.joyride-tip-guide.bordered span.joyride-nub.top::after {
top: -16px;
left: -15px;
border: 15px solid #808080 !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all of these !important declarations necessary? These are pretty specific selectors already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, that's a good point. On the after elements, !important should not be needed. I will double-check and remove.

@@ -5,7 +5,7 @@
<p class="splashwelcome">
This application allows you to interactively browse global satellite imagery within hours of it being acquired. Use the features described below to find interesting imagery, save and share what you find, and download the underlying data.
</p>
<br>
<br><br>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not quite sure what caused the extra spacing in production, but I think a single space is fine, and fits the rhythm of the page better. A double <br><br> seems unnecessary. What do you think about removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I added it back in for parity with what we had before.

@ZachTRice ZachTRice force-pushed the module-loaders-fix-style-regressions branch from 7933f55 to ddfbc80 Compare January 18, 2018 14:29
- Add "Segoe UI" default font back to "Add Layers" button
- Change tour step 1 to be a wide window
- Add `bordered` styles to tour windows which were removed from our version of joyride;
this requires using an after element on the `nub` element whereas before there was a second element.
- Fix margins in tour windows
- Remove `minimal-ui` viewport attribute
Connects to #671, Connects to #670
@ZachTRice ZachTRice force-pushed the module-loaders-fix-style-regressions branch from ddfbc80 to 739e335 Compare January 18, 2018 15:06
Copy link
Contributor

@localjo localjo left a comment

Choose a reason for hiding this comment

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

Looks good

@ZachTRice ZachTRice merged commit aaa53fd into module-loaders Jan 18, 2018
@ghost ghost removed the ready for review label Jan 18, 2018
@ZachTRice ZachTRice deleted the module-loaders-fix-style-regressions branch January 18, 2018 15:13
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