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

remove jquery from job log viewer, other minor fixes #1623

Merged
merged 4 commits into from
Apr 8, 2020

Conversation

briehl
Copy link
Member

@briehl briehl commented Apr 7, 2020

Addresses #1621

  • Make unique element id for viewer with html.genId() (see line 1042 - fetching the panel element)
    • set id = the generated id
  • Remove most/all references to jquery and jquery as a dependency
    • There was just one remaining - an animate step. Swapped this out with a simple CSS transition.
  • Rename warpperDiv -> wrapperDiv
    • just a typo, fixed
  • Redundant if statements around line 663.
    • Also fixed.

@coveralls
Copy link

coveralls commented Apr 7, 2020

Coverage Status

Coverage increased (+0.2%) to 17.55% when pulling 0506272 on briehl:log-viewer into eb563fa on kbase:develop.

@briehl
Copy link
Member Author

briehl commented Apr 7, 2020

Also fixes #1620

@eapearson
Copy link
Contributor

Looks good to me, code wise, nice cleanup.
A local build, launch, and use of the narrative went fine, other than the build and launch issues noted elsewhere which (a) are noted elsewhere and (b) didn't affect opening a narrative, running a job, and observing and using the job log browser.
Job log browser behaved as expected, modulo other bugs to fix in future PRs.
The line indenting and line mixup are fixed too, nice :)

@briehl
Copy link
Member Author

briehl commented Apr 8, 2020

Hm, I suspect the line mixup isn't fixed yet - I haven't done anything against that. I think it's an async issue.
But I'll go ahead and merge at this point. :)

@briehl briehl merged commit eb7f02e into kbase:develop Apr 8, 2020
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.

3 participants