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

Adding loading component #108

Merged
merged 4 commits into from
Aug 20, 2017
Merged

Adding loading component #108

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89b137b
loading component added for both detail and list
lazywithclass Aug 19, 2017
c7bd6cc
removed old loader and related styles
lazywithclass Aug 19, 2017
33b0cb3
reintroduced dotdotdot class and moved loader to the center
lazywithclass Aug 20, 2017
fd327cf
using the correct syntax for import
lazywithclass Aug 20, 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 .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,4 @@ archives/.idea/
.env
.vscode/
.DS_Store
dist
7 changes: 7 additions & 0 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ gulp.task('css', function () {
return gulp.src('src/css/*').pipe(gulp.dest('dist/css'));
});

gulp.task("default", ["css", "img", "browserify"]);

gulp.task("img", function () {
return gulp.src('src/img/*').pipe(gulp.dest('dist/img'));
});

gulp.task('browserify', function () {
return browserify({
basedir: '.',
Expand All @@ -27,4 +33,5 @@ gulp.task('browserify', function () {
gulp.task('watch', function() {
gulp.watch('src/**/*.{js,jsx}', ['browserify']);
gulp.watch('src/**/*.css', ['css']);
gulp.watch('src/img/*', ['img']);
});
12 changes: 12 additions & 0 deletions src/components/loading.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use import React from 'react' here. #101 explains why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure


export default class Loading extends React.Component {

constructor () {
super();
}

render () {
return (<div className="loading">&nbsp;</div>)
}
}
4 changes: 2 additions & 2 deletions src/components/page-details.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react';
import {Link, Redirect} from 'react-router-dom';
import WebMonitoringDb from '../services/web-monitoring-db';
import ChangeView from './change-view';
import Loading from './loading'

/**
* @typedef {Object} PageDetailsProps
Expand Down Expand Up @@ -70,9 +71,8 @@ export default class PageDetails extends React.Component {
}

render () {
// TODO: should factor out a loading view
if (!(this.state.page)) {
return <div>Loading…</div>;
return (<Loading />);
}

const page = this.state.page;
Expand Down
3 changes: 2 additions & 1 deletion src/components/page-list.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import Loading from './loading'

/**
* These props also inherit from React Router's RouteComponent props
Expand All @@ -16,7 +17,7 @@ import React from 'react';
export default class PageList extends React.Component {
render () {
if (!this.props.pages) {
return <div>Loading…</div>;
return (<Loading />);
}

return (
Expand Down
33 changes: 7 additions & 26 deletions src/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,32 +297,6 @@ iframe {
width: 100%;
}

/* ===== Progress Bar ===== */
.progress {
width: 100%;
}

.progress .progress-bar.active {
font-weight: 700;
animation: progress-bar-stripes .5s linear infinite;
}

.dotdotdot:after {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep .dotdotdot class and animation. It's still useful for text as mentioned in #107

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

font-weight: 300;
content: '...';
display: inline-block;
width: 20px;
text-align: left;
animation: dotdotdot 1.5s linear infinite;
}

@keyframes dotdotdot {
0% { content: '...'; }
25% { content: ''; }
50% { content: '.'; }
75% { content: '..'; }
}

/* ===== Login Form ===== */
.dialog__body {
background: white;
Expand All @@ -345,3 +319,10 @@ iframe {
.login-form__footer {
text-align: right;
}

.loading {
margin-left: 20px;
height: 70px;
background-image: url(../img/infinity-loader.svg);
background-repeat: no-repeat;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add background-position-x: center instead of using margin-left? Just preferred aesthetically.

Copy link
Contributor Author

@lazywithclass lazywithclass Aug 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, I'm a terrible noob when talking CSS, could you please share a link about the differences?

Aaah, I see, it moves the loading image at the center, ok!

}
5 changes: 5 additions & 0 deletions src/img/infinity-loader.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.
12 changes: 1 addition & 11 deletions views/main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,7 @@
<script src="https://use.fontawesome.com/2155f7b139.js"></script>
</head>
<body id="bootstrap-override">
<div id="web-monitoring-ui-root">
<div class="row">
<div class="col-md-12">
<div class="progress active progress-striped">
<div class="progress-bar progress-success" style="width:100%" role="progressbar" aria-valuemin="0" aria-valuemax="100" aria-valuenow="100">
<span>Please wait<span class="dotdotdot"></span></span>
</div>
</div>
</div>
</div>
</div>
<div id="web-monitoring-ui-root"></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should replace that old div.row with div.loading here, too.

Copy link
Contributor Author

@lazywithclass lazywithclass Aug 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't get this one, sorry, I removed the old loading and left it as it was because when loading the page the new loader appears; what should I do?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a short amount of time before the React app renders, so I was suggesting that a div.loading was placed in the root div where the other one was before. It gets quickly replaced anyway, so not a big deal, just me being fussy. I'm working on some loading stuff and I really want to use your loading component, so I'll merge and just add this myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I see :)


<script type="text/javascript" src="/bundle.js"></script>
</body>
Expand Down