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

feat(cli): better loading ui #800

Merged
merged 4 commits into from
Aug 4, 2020
Merged

feat(cli): better loading ui #800

merged 4 commits into from
Aug 4, 2020

Conversation

PatrickJS
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?
Better loading ui indicator

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Other... Please describe:

What is the current behavior?

loading indicator doesn't really show whats going on

Issue Number: N/A

What is the new behavior?

more loading indicators

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

fixes #654

logWarn(`
----------------
Error, missing "${yellow('<scully-content>')}" in route "${yellow(route.route)}"
without <scully-content> we can not render this route.
Make sure it is in there, and not inside any conditionals (*ngIf)
You can check this by opening "${yellow(`http${ssl ? 'S' : ''}://localhost:4200/${route.route}`)}"
You can check this by opening "${yellow(`http${ssl ? 'S' : ''}://localhost:4200/${route.route.replace(/^\//, '')}`)}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

route.route sometimes starts with /

@@ -14,12 +15,16 @@ export function injectHtml(html: string, additionalHTML: string, route: HandledR
try {
attr = getIdAttrName(html.split('<scully-content')[1].split('>')[0].trim());
} catch (e) {
// clear any progress
if (process.stdout.cursorTo) {
readline.clearLine(process.stdout, 0);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is just to clear out any loading indicators

printProgress(state.count, 'Total Routes');
setTimeout(() => {
resolve(data);
}, 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you can change this to 100 to see "loading" of when its reading the directories

@@ -21,20 +21,35 @@ export async function contentFolderPlugin(angularRoute: string, conf: RouteTypeC
}
const baseRoute = angularRoute.split(':' + param)[0];
basePath = join(scullyConfig.homeFolder, paramConfig.folder);
const state = { count: 0 };
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

used a global shared state rather than getting fancy

createFolderFor(file);
printProgress(i + 1, 'Creating Route List:', collection.length);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this will be better when its write async rather than sync

@@ -21,7 +21,8 @@ export async function asyncPool<T>(MaxParralellTasks: number, array: T[], taskFn
executing.push(e);
const now = performance.now();
if (now - logTime > progressTime) {
printProgress(Math.max(array.length - ret.length, executing.length));
const tasksLeft = Math.max(array.length - ret.length, executing.length);
printProgress(array.length + 1 - tasksLeft, 'Rendering Routes:', array.length);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

counting up rather than down

@@ -17,6 +17,9 @@ registerPlugin(scullySystem, generateAll, plugin);
async function plugin(localBaseFilter = baseFilter): Promise<HandledRoute[]> {
await loadConfig;
try {
// maintain progress ui
startProgress();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we need a way to maintain state within logging during this time

Copy link
Contributor

@jorgeucano jorgeucano left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@SanderElias SanderElias left a comment

Choose a reason for hiding this comment

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

Did some refactoring to make display of the spinner more "smooth"
also simplified the updating in contentFolder

@SanderElias SanderElias merged commit 0141e26 into main Aug 4, 2020
@SanderElias SanderElias deleted the feat-better-loading-ui branch August 4, 2020 14:48
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.

loading indicator for case when it's a lot of md files to compile
3 participants