Styles updates, Theming and other misc updates #195

aulneau · 2017-12-31T22:18:17Z

Okay, I've gone through just about every style in this thing and updated / refactored them to be cleaner and use variables whenever possible.

I've also added a new step to our css, I've added postcss to process our generated css and do a few different things. It seemed like we weren't autoprefixing our css at all, so I've added autoprefixer. I've also opted to use cssnano with postcss to minify our css in that process, so we are no longer using the node-sass compress functionality. I've added a postcss plugin to combine duplicate selectors into one.

For IE11, I've added a couple postcss plugins to convert all of our custom css-props to their true values, so that if people need to support IE11 (eg: chart studio), you just include the ie11 stylesheet instead. This step can also be integrated into the users app, so that if they wanted to theme the editor first, and then run the postcss plugin themselves, they could. I've also added a plugin to remove :root{} from the stylesheet if you are generating styles for ie11.

I've also added an empty state for if there is no trace data, the message could/should be modified:

I have added a delete component within the field component:

I've also added a Button component.

I still need to go through and update some icons to what we have in plotly-icons, but I wanted to get this PR open :)

Regarding theming: I've opted to use scss maps and then parse through them to generate our scss variables.

The various scss maps:

are mapped through with scss functions:

which then are outputted as css variables:

which can be easily overridden by the end user in their app:

which allows for a themable app:

resolves: #184 #183 #76

… works for defaults

…ed to a variable

bpostlethwaite · 2018-01-01T18:54:58Z

Awesome work! I haven't looked in depth at this yet and maybe @VeraZab can do a review of the CSS?
As part of this PR could you add language to the README that tells people how they would customize this library? That and the build process are what I will focus on (the style and build API for developers).

Also AFAIK you are not restricted to NODE_ENV. You could, for example, use another env variable called PLOTLYJS_EDITOR_CSS_TARGET and then set that in the build command before running the javascript that does the building.

NODE_ENV is usually used to distinguish between 'dev' and 'production' builds.

bpostlethwaite · 2018-01-01T18:56:58Z

Also this PR seems to address more than one existing issue. I see "empty state for no traces" as a commit message for example. Could you update your description to include a "resolves #x #y and #z" type declaration? Github will auto-close the issues and it will give me more context. Thanks!

VeraZab

💃 except for using icons from plotly-icons instead of mdi :)

VeraZab · 2018-01-02T16:08:36Z

package.json

    "fast-isnumeric": "^1.1.1",
+    "mdi-react": "^2.1.19",
    "plotly-icons": "^1.0.1",


@aulneau you're going to add the additional mdi-react icons we need to plotly-icons?

VeraZab · 2018-01-02T16:12:13Z

src/components/containers/TraceAccordion.js

+        className="js-add-trace-button"
+        variant="primary"
+        onClick={this.addTrace}
+        icon={<PlusIcon />}


or PlusIcon could be the default in the Button component?

or maybe not, if we're including modals to the editor (which I'm not sure we will be), maybe we want more flexibility on that button

Maybe... I think it might be better to leave no default icon, because there are buttons that don't have icons.

VeraZab · 2018-01-02T16:16:39Z

src/components/sidebar/SidebarGroup.js

-import {bem} from '../../lib';
-
+import {bem} from 'lib';
+import ChevronRightIcon from 'mdi-react/ChevronRightIcon';
 import SidebarItem from './SidebarItem';


it's AngleDownIcon in plotly-icons

Cool, I'll make sure to update all of the icons throughout with our plotly-icons and add any I use throughout :)

VeraZab · 2018-01-02T16:21:44Z

src/styles/_helpers.scss

+    @return map-get($spacings,$name);
+  } @else {
+    @return var(--spacing-#{$name});
+  }


is this a sass thing: var(--spacing-#{$name}) how does it work?

whoops, these are no longer used, I'll remove them.

I've added a lot of scss maps and parsers, to out put css custom properties (native css variables) from our scss.

VeraZab · 2018-01-02T16:23:37Z

src/styles/_mixins.scss

@@ -34,7 +34,7 @@
  }
 }

-@mixin scrollbar($width: 5px, $trackbg: $color-page-background, $thumb-color: $color-accent) {
+@mixin scrollbar($width: 5px, $trackbg: var(--scrollbar-track-background), $thumb-color: var(--scrollbar-thumb-color)) {
  -webkit-overflow-scrolling: touch;


what's the difference here between $color-accent and var(--scrollbar-thumb-color)

So a lot of these variables are kind of passed around so that you can theme the application, the reason this is set this way is so that the end user can change the accent color, and the scollbar default thumb color would be the accent color, but if they wanted a different color for the scrollbar, they would change the value of --scrollbar-thumb-color.

VeraZab · 2018-01-02T17:45:31Z

haha, yeah..at some point we're going to have to clean up some of those scripts in the editor, there are soooo many, but yeah, not in this pr : )

aulneau · 2018-01-02T17:48:03Z

Yeah I know :/ I felt so bad making like 6 more scripts in there... There has to be a way to abstract and make it more efficient.

bpostlethwaite · 2018-01-02T18:50:51Z

src/components/containers/TraceAccordion.js

+          message={
+            <p>
+              Looks like there aren't any traces defined yet. Go to the 'Create'
+              tab to define some traces.


this needs to be wrapped in i18n _(). Ping me or @VeraZab with any questions

bpostlethwaite · 2018-01-02T18:51:10Z

src/components/containers/TraceAccordion.js

+        variant="primary"
+        onClick={this.addTrace}
+        icon={<PlusIcon />}
+        label="Trace"


this needs to be wrapped in i18n

bpostlethwaite · 2018-01-02T18:51:41Z

src/components/containers/AnnotationAccordion.js

+        className="js-add-annotation-button"
+        onClick={this.addAnnotation}
+        icon={<PlusIcon />}
+        label="Annotation"


bpostlethwaite · 2018-01-02T18:53:52Z

src/components/Button.js

@@ -0,0 +1,44 @@
+import PropTypes from 'prop-types';


Button should go in components/widgets?

bpostlethwaite · 2018-01-02T18:55:03Z

src/styles/components/_button.scss

@@ -0,0 +1,73 @@
+// button default styles


this should be within a folder in components/? Seems like a widget to me.

…sparsely

…e more theme related variables

aulneau · 2018-01-03T17:25:34Z

Okay @bpostlethwaite and @VeraZab, I've updated the readme, added a theming document, and I believe I've fixed everything you two have requested. If you can review my latest changes, I'd say this is good to go! :) (barring any new changes requested)

VeraZab · 2018-01-03T17:32:19Z

yey 💃

bpostlethwaite · 2018-01-03T17:57:56Z

package.json

-      "mkdirp dist && browserify src/PlotlyEditor.js -o ./dist/PlotlyEditor.js -t [ babelify --presets [ es2015 react ] ] -t browserify-global-shim --standalone createPlotlyComponent && uglifyjs ./dist/PlotlyEditor.js --compress --mangle --output ./dist/PlotlyEditor.min.js --source-map filename=dist/PlotlyEditor.min.js.map && make:dist:css",
-    "make:dist:css":
-      "mkdirp dist && node-sass --output-style compressed src/styles/main.scss > dist/react-plotly.js-editor.css",
+    "make:lib": "mkdirp lib && babel src --out-dir lib --ignore=__tests__/* --source-maps && npm run make:lib:css",


@VeraZab I think you and @aulneau have squabbling .json formatting. I don't think we should use prettier on anything outside /src or at least not on anything other than *.js files.

Yeah, sometimes my muscle memory causes me to autoformat anything after I hit save. But it's strange, my current package.json is formatted correctly:

bpostlethwaite · 2018-01-03T17:59:31Z

thanks for the heroic level of effort here! Can't wait to try out some new themes!

💃

VeraZab · 2018-01-03T18:01:54Z

new release after this merge! : )

aulneau · 2018-01-03T18:14:19Z

@VeraZab okay I fixed the prepublishOnly task. should be good to go! (also I don't have permission to merge this PR)

aulneau added 18 commits December 28, 2017 13:21

update styles for the sidebar, added chevron, more dynamic states

4b4cebc

minor update to modal info styles

44dc991

start migrating over to custom-vars, added postcss step for ie11 that…

8784cb4

… works for defaults

fixes to button styles when using standard html button

e3c8125

update from master

8f63761

most variables now exist as css props

5366cec

massive change to variables, almost everything (hopefully) is connect…

7a90ba2

…ed to a variable

fix buttons for tests

f732a3b

go through fold styles

21b92d4

go through info and menu panel styles

99326af

update modalbox, radioblock, menupanel, section and field styles

afc48ff

update field styles, added FieldDelete element

34d688c

update color picker styles

877c2a0

update sidebar styles, misc others

1274aa7

update checkbox styles

fcea940

updates for theming, updated postcss settings

083232c

further theme tweaks

9bce0eb

add empty state for no traces

4f36879

aulneau requested review from bpostlethwaite and VeraZab December 31, 2017 22:18

VeraZab mentioned this pull request Jan 2, 2018

Add streambed svg icons plotly/plotly-icons#3

Merged

VeraZab suggested changes Jan 2, 2018

View reviewed changes

add scripts, updated package.json with new scripts. IE css works.

00f8ff6

update scripts location, remove outdated style

0bf806d

bpostlethwaite reviewed Jan 2, 2018

View reviewed changes

aulneau added 8 commits January 2, 2018 13:04

move button into widgets, added font-family to .plotly-editor

fd1f37d

remove relative 'lib', also small radiobutton css fix

0f5a991

localize annotation and trace accordians

135bbf0

fix changed path

67cf1da

added plotly fonts, added header style mixin, applied heading styles …

02a655b

…sparsely

removed mdi-react dep, updated icons to plotly-icons

9d91096

update styles scripts to generate sep stylesheets for ie and modern

41a7942

update README.md, added THEMING.md, clean up colors and added a coupl…

e8b2b42

…e more theme related variables

VeraZab previously approved these changes Jan 3, 2018

View reviewed changes

bpostlethwaite reviewed Jan 3, 2018

View reviewed changes

fix prepublishOnly task, fixed DeprecationWarning in the styles scripts

4742c0d

aulneau dismissed VeraZab’s stale review via 4742c0d January 3, 2018 18:12

VeraZab approved these changes Jan 3, 2018

View reviewed changes

Merge branch 'master' into further-design-updates

cfa1536

aulneau merged commit 50ea184 into master Jan 3, 2018

This was referenced Jan 3, 2018

zero state handling #76

Closed

CSS - allow multiple widgets in widget field #183

Closed

MultiTextEditor bold not showing up as bold in TextArea #147

Closed

aulneau deleted the further-design-updates branch January 8, 2018 23:02

Uh oh!

Styles updates, Theming and other misc updates #195

Styles updates, Theming and other misc updates #195

Uh oh!

Conversation

aulneau commented Dec 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpostlethwaite commented Jan 1, 2018

Uh oh!

bpostlethwaite commented Jan 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VeraZab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VeraZab commented Jan 2, 2018

Uh oh!

aulneau commented Jan 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aulneau commented Jan 3, 2018

Uh oh!

VeraZab commented Jan 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpostlethwaite commented Jan 3, 2018

Uh oh!

VeraZab commented Jan 3, 2018

Uh oh!

aulneau commented Jan 3, 2018

Uh oh!

Uh oh!

aulneau commented Dec 31, 2017 •

edited

Loading

bpostlethwaite commented Jan 1, 2018 •

edited

Loading