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

10905 layer onboarding #11532

Merged
merged 58 commits into from Feb 20, 2017
Merged

10905 layer onboarding #11532

merged 58 commits into from Feb 20, 2017

Conversation

ivanmalagon
Copy link
Contributor

@ivanmalagon ivanmalagon commented Feb 14, 2017

Implements #10905

This PR enables a user onboarding flow when entering the layer edition.

onboarding

Minimum Viable Acceptance

There are three distinct onboardings when entering layer edition:

  1. Data tab.
  2. Analysis tab.
  3. Style tab.

There are plenty of combinations per each one.

Data tab

There are four steps in the onboarding process. The last step is the tricky one since its location depends of the map state.

The different cases to tests are:

  • No widgets.
  • With side widgets.
  • With time series widget.
  • With animated time series widget.

And per each one there are three responsive breakpoints that modify page elements sizes.

  • More than 1600px width. (1680x1050, for instance).
  • More than 1400px width. (1440x900, for instance).
  • Less than 1400px width. (1024x768, for instance).

Known issues
The buttons for geometry edition and dataset switching doesn't place themselves in the right position in the smaller breakpoint. The onboarding highlights the right place where they should be placed once this issue is fixed.

Analysis tab

Only one step. Test that the button triggers an analysis creation.

Style tab

Two different scenarios.

  1. Points geometry: in this scenario test all different aggregations (simple, heatmap, hexabins, ...) since there are different sizes for the menu options that reflect on this onboarding.

  2. Lines or polygons geometry: one less step than in points onboarding.

The onboarding was already checked and accepted by the one and only @arianaescobar and @noguerol will take a look also, as far as I know.

Warning

If you check Don't show this again you won't see the onboarding (that's how it works 😉 ) so test those checks as the last step.

If you accidentally check them and you need to trigger the onboarding againg, ping me.

&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px);
}
&.has-widgets {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

}
&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px);
}

Choose a reason for hiding this comment

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

Line contains trailing whitespace

&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px); // 151 - XL animated
}
&.has-widgets {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

}
&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px); // 151 - XL animated
}

Choose a reason for hiding this comment

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

Line contains trailing whitespace

&.has-timeSeries {
height: 151px;
}
&.has-timeSeries--animated {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

.LayerOnboarding-padBottom {
height: 16px;

&.has-timeSeries {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

&.has-timeSeries {
height: calc(100% - (64 + 151) * 1px); // 151 - XXXL time serie
}
&.has-timeSeries--animated {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

.LayerOnboarding-padTop {
height: calc(100% - (64 + 16) * 1px);

&.has-timeSeries {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px);
}
&.has-widgets {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

}
&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px);
}

Choose a reason for hiding this comment

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

Line contains trailing whitespace

&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px); // 151 - XL animated
}
&.has-widgets {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

}
&.has-timeSeries--animated {
height: calc(100% - (64 + 151) * 1px); // 151 - XL animated
}

Choose a reason for hiding this comment

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

Line contains trailing whitespace

&.has-timeSeries {
height: 151px;
}
&.has-timeSeries--animated {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

.LayerOnboarding-padBottom {
height: 16px;

&.has-timeSeries {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

&.has-timeSeries {
height: calc(100% - (64 + 151) * 1px); // 151 - XXXL time serie
}
&.has-timeSeries--animated {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

.LayerOnboarding-padTop {
height: calc(100% - (64 + 16) * 1px);

&.has-timeSeries {

Choose a reason for hiding this comment

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

Nesting should be no greater than 3, but was 4

}

.LayerOnboarding--data .LayerOnboarding-pads--right {
width: 136px;

Choose a reason for hiding this comment

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

Properties should be ordered display, width

.LayerOnboarding-padMiddle,
.LayerOnboarding-padBottom {
@include transition(height, 300ms, ease-in-out);
width: 100%;

Choose a reason for hiding this comment

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

Properties should be ordered display, width

}

.LayerOnboarding-widgetsOverlay {
width: 16px;

Choose a reason for hiding this comment

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

Properties should be ordered display, width

width: $sBigDescriptionWidth;
}

.LayerOnboarding-description.is-step4{

Choose a reason for hiding this comment

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

Opening curly brace { should be preceded by one space

margin-bottom: 16px;
}

.LayerOnboarding-headerText.is-step4{

Choose a reason for hiding this comment

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

Opening curly brace { should be preceded by one space


.LayerOnboarding-headerText {
width: $sBigDescriptionWidth;
color: #FFF;

Choose a reason for hiding this comment

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

Properties should be ordered width, margin-top, margin-bottom, color, font-size, line-height


.LayerOnboarding-body.is-step4 {
right: 100%;
left: auto;

Choose a reason for hiding this comment

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

Properties should be ordered right, bottom, left

}
}, this);
AnalysesService.addAnalysis(layerId);
// this._onboardings.destroy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Ouch, I've forgotten to remove this commented code please 💋 .

@matallo
Copy link
Contributor

matallo commented Feb 15, 2017

@ivanmalagon spotted this just after creating a map, I'll show you tomorrow

130b1873-2bfa-43fb-9a44-59fbf14aaeb9

numberOfSteps: 4,
modifier: '--data'
}));
if (opts.numberOfWidgets === void 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are not we using the helper for the required options anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@xavijam xavijam left a comment

Choose a reason for hiding this comment

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

🇪🇸

@javierarce javierarce merged commit 005cd0b into master Feb 20, 2017
@javierarce javierarce deleted the 10905-layer-onboarding branch February 20, 2017 08:57
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.

None yet

6 participants