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.

Sign up for GitHub

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

Add config option to display a clean onboarding look to users. #580

Merged
merged 2 commits into from
Nov 16, 2017
Merged

Add config option to display a clean onboarding look to users. #580

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccde896
Add config option to display a clean onboarding look to users.
Nov 15, 2017
2070d93
Minor CR fixups
robertknight Nov 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/annotator/config/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function configFrom(window_) {
disableToolbarHighlightsBtn: settings.hostPageSetting('disableToolbarHighlightsBtn'),
disableToolbarNewNoteBtn: settings.hostPageSetting('disableToolbarNewNoteBtn'),
disableBucketBar: settings.hostPageSetting('disableBucketBar'),
enableCleanOnboardingTheme: settings.hostPageSetting('enableCleanOnboardingTheme'),
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps for both consistency with how we are handling the "New note" button and also to make it clear that this isn't yet an officially supported setting, we ought to name it something like enableExperimentalNewOnboarding? 🤔 .

The new-look onboarding actually works fine whether or not the "clean" theme is being used, so perhaps we can consider it a separate thing.

enableExperimentalNewNoteButton: settings.hostPageSetting('enableExperimentalNewNoteButton'),
enableSidebarDropShadow: settings.hostPageSetting('enableSidebarDropShadow'),
theme: settings.hostPageSetting('theme'),
Expand Down
14 changes: 14 additions & 0 deletions src/images/icons/cursor.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion src/sidebar/components/sidebar-tutorial.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use strict';

// @ngInject
function SidebarTutorialController(session) {
function SidebarTutorialController(session, settings) {
this.cleanOnboardingThemeEnabled = settings.enableCleanOnboardingTheme;

this.showSidebarTutorial = function () {
if (session.state.preferences) {
if (session.state.preferences.show_sidebar_tutorial) {
Expand Down
1 change: 1 addition & 0 deletions src/sidebar/components/svg-icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// The list of supported icons
var icons = {
refresh: require('../../images/icons/refresh.svg'),
cursor: require('../../images/icons/cursor.svg'),
};

// @ngInject
Expand Down
10 changes: 6 additions & 4 deletions src/sidebar/components/test/sidebar-tutorial-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ var Controller = require('../sidebar-tutorial').controller;
describe('SidebarTutorialController', function () {

describe('showSidebarTutorial', function () {
var settings = { enableCleanOnboardingTheme: true };

it('returns true if show_sidebar_tutorial is true', function () {
var session = {
state: {
Expand All @@ -13,7 +15,7 @@ describe('SidebarTutorialController', function () {
},
},
};
var controller = new Controller(session);
var controller = new Controller(session, settings);

var result = controller.showSidebarTutorial();

Expand All @@ -28,7 +30,7 @@ describe('SidebarTutorialController', function () {
},
},
};
var controller = new Controller(session);
var controller = new Controller(session, settings);

var result = controller.showSidebarTutorial();

Expand All @@ -37,7 +39,7 @@ describe('SidebarTutorialController', function () {

it('returns false if show_sidebar_tutorial is missing', function () {
var session = {state: {preferences: {}}};
var controller = new Controller(session);
var controller = new Controller(session, settings);

var result = controller.showSidebarTutorial();

Expand All @@ -46,7 +48,7 @@ describe('SidebarTutorialController', function () {

it('returns false if session.state is {}', function () {
var session = {state: {}};
var controller = new Controller(session);
var controller = new Controller(session, settings);

var result = controller.showSidebarTutorial();

Expand Down
3 changes: 3 additions & 0 deletions src/sidebar/host-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ function hostPageConfig(window) {
// This should be removed once new note button is enabled for everybody.
'enableExperimentalNewNoteButton',

// New onboarding theme override.
'enableCleanOnboardingTheme',

// OAuth feature flag override.
// This should be removed once OAuth is enabled for first party accounts.
'oauthEnabled',
Expand Down
35 changes: 34 additions & 1 deletion src/sidebar/templates/sidebar-tutorial.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="sheet" ng-if="vm.showSidebarTutorial()">
<div class="sheet" ng-if="vm.showSidebarTutorial() && !vm.cleanOnboardingThemeEnabled">
<i class="close h-icon-close" role="button" title="Close"
ng-click="vm.dismiss()"></i>
<h1 class="sidebar-tutorial__header">How to get started</h1>
Expand Down Expand Up @@ -41,3 +41,36 @@ <h1 class="sidebar-tutorial__header">How to get started</h1>
</li>
</ol>
</div>
<div class="sheet sheet--is-theme-clean" ng-if="vm.showSidebarTutorial() && vm.cleanOnboardingThemeEnabled">
<i class="close h-icon-close" role="button" title="Close"
ng-click="vm.dismiss()"></i>
<h1 class="sidebar-tutorial__header sidebar-tutorial__header--is-theme-clean">
<i class="h-icon-annotate sidebar-tutorial__header-annotate" h-branding="accentColor"></i>
Start annotating
</h1>
<ol class="sidebar-tutorial__list">
<li class="sidebar-tutorial__list-item sidebar-tutorial__list-item--is-theme-clean">
<div class="sidebar-tutorial__list-item-content">
Select some text to
<span class="sidebar-tutorial__list-item-annotate">annotate</span>
<svg-icon width="3px" height="10px" class="sidebar-tutorial__list-item-cursor" name="'cursor'"></svg-icon>
Copy link
Member

Choose a reason for hiding this comment

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

The width and height properties here are redundant and can be removed since we're now applying the styling in CSS.

or highlight.
</div>
</li>
<li class="sidebar-tutorial__list-item sidebar-tutorial__list-item--is-theme-clean">
<div class="sidebar-tutorial__list-item-content sidebar-tutorial__list-item-content--is-theme-clean">
Create page level notes
<button class="sidebar-tutorial__list-item-new-note-btn" h-branding="ctaBackgroundColor">
Copy link
Member

Choose a reason for hiding this comment

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

Hmm ... this isn't actually clickable so it might be a bit surprising that it has the affordances of a button (eg. a cursor pointer).

+ New note
</button>
</div>
</li>
<li class="sidebar-tutorial__list-item sidebar-tutorial__list-item--is-theme-clean">
<div class="sidebar-tutorial__list-item-content">
View annotations through your profile
<i class="h-icon-account sidebar-tutorial__list-item-profile"></i>
<i class="h-icon-arrow-drop-down sidebar-tutorial__list-item-drop-down"></i>
</div>
</li>
</ol>
</div>
8 changes: 8 additions & 0 deletions src/styles/app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ hypothesis-app {
}
}

.sheet--is-theme-clean {
padding-left: 30px;
padding-bottom: 30px;
margin-bottom: 20px;
margin-left: 5px;
margin-right: 5px;
}

.annotation-unavailable-message {
display: flex;
flex-direction: column;
Expand Down
56 changes: 56 additions & 0 deletions src/styles/sidebar-tutorial.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@
font-weight: $title-font-weight;
}

.sidebar-tutorial__header-annotate {
position: relative;
top: 2px;
}

.sidebar-tutorial__header--is-theme-clean {
color: $gray-dark;
font-size: 16px;
}

/* We want an ordered list in which the numbers are aligned with the text
above (not indented or dedented), and wrapped lines in list items are
aligned with the first character of the list item (not the number, i.e
Expand Down Expand Up @@ -44,10 +54,56 @@
color: $gray-light;
}

.sidebar-tutorial__list-item--is-theme-clean {
font-size: 14px;
}

.sidebar-tutorial__list-item--is-theme-clean::before {
color: $gray-dark;
}

.sidebar-tutorial__list-item-content {
margin-top: 1em; /* Put vertical space in-between list items, equal to the
space between normal paragraphs.
Note: This also puts the same amount of space above the
first list item (i.e. between the list and whatever's
above it). */
}

.sidebar-tutorial__list-item-annotate {
background-color: $highlight-color-focus;
padding: 0px 3px;
}

.sidebar-tutorial__list-item-new-note-btn {
background-color: #626262;
Copy link
Member

Choose a reason for hiding this comment

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

This should use the $color-dove-gray variable for consistency with other elements that are styled using the same palette element from the color palette.

border: none;
border-radius: 3px;
color: #fff;
font-weight: 500;
text-align: center;
margin-left: 2px;
}

.sidebar-tutorial__list-item-drop-down {
margin-left: -5px;
}

.sidebar-tutorial__list-item-cursor {
position: relative;
top: 3px;
margin-left: -10px;
}

.sidebar-tutorial__list-item-cursor svg {
width: 12px;
height: 17px;
}

.sidebar-tutorial__list-item-content {
margin-top: 16px;
}

.sidebar-tutorial__list-item-profile {
margin-left: 4px;
}