NetSim: Reactified Log Browser#9721
Merged
Merged
Conversation
| &.sort-asc:before { | ||
| content: '\F0DE'; | ||
| } | ||
| &.sort-desc:before { |
There was a problem hiding this comment.
Begin pseudo elements with double colons: ::
Puts back a bunch of code I removed to get the old version working again.
|
|
||
| describe("Log Mode", function () { | ||
| var modal, rootDiv, testShard, router, levelConfig; | ||
| testUtils.forEveryBooleanPermutation(useNewLogBrowser => { |
Contributor
Author
There was a problem hiding this comment.
Only change here is to run most tests in this file both with the experiment enabled and disabled. Recommended to view without whitespace changes.
Also get everything working nicely in storybook again, with neat randomly-generated sample data.
| * @overview a modal dialog showing the union of all router logs for the | ||
| * current shard. | ||
| */ | ||
| 'use strict'; |
Contributor
Author
There was a problem hiding this comment.
Now that we use Babel and treat everything as ES6, this is redundant because ES6 modules are always in strict mode.
Actually, I should do a cleanup PR that goes through and removes 'use strict' throughout our JavaScript.
| render() { | ||
| return ( | ||
| <Dialog fullWidth {...this.props}> | ||
| <Title>Log Browser</Title> |
Contributor
There was a problem hiding this comment.
should this be internationalized?
Contributor
Author
There was a problem hiding this comment.
I totally forgot to do this - there's some actual functionality around the title reflecting the current filter settings. I'll port it over.
Contributor
|
a few minor comments and questions, but this otherwise looks great! I'm excited to see the code removal PR! |
...including changes to reflect current filter settings.
Replaced with a custom reactabular cell transform wrapper to add the correct sort icons to the header cells.
Contributor
Author
|
PTAL |
Contributor
|
This looks fantastic! |
Contributor
Author
|
I'll take that as a LGTM. |
deploy-code-org
added a commit
that referenced
this pull request
Jul 29, 2016
26780fa Merge pull request #9721 from code-dot-org/netsim-teacher-view (Brad Buchanan) 751ca8a Automatically built. (Continuous Integration) da1d1b8 Merge pull request #9748 from code-dot-org/bump-block-svg-framed (Elijah Hamovitz) f646303 Fix tests by passing locale into component (Brad Buchanan) 5bf6038 Merge pull request #9750 from code-dot-org/fix-topinstructions-feature-test (Elijah Hamovitz) 4de1a32 Removes last .new-router-log-modal style (Brad Buchanan) 2821f6e Move most styles into JSX (Brad Buchanan) 5f3e688 Merge pull request #9739 from code-dot-org/hideStagesNonAuthorized2 (Brent Van Minnen) d327da6 move uitest-specific className from custom Component which does not support classnames to standard paragraph tag which does (Elijah Hamovitz) c53f0ac Use px for filters marginBottom (Brad Buchanan) 31be25a Move sort methods onto the component (Brad Buchanan) 952a54e Indent conditional component (Brad Buchanan) 50cd059 Restore localized log browser header (Brad Buchanan) e38b4f8 Manually specify blockSvgFramed clippath width (Elijah Hamovitz) cf3c84e Merge pull request #9742 from code-dot-org/topInstructions-fix-unwanted-dashed-border (Elijah Hamovitz) 06d30ae Merge pull request #9744 from code-dot-org/fix-survey-view (Brendan Reville) 9b03d61 Merge pull request #9745 from code-dot-org/levelbuilder (Josh Lory) 26501e2 Level builder changes (Continuous Integration) 0cbfc1e Content changes (Continuous Integration) 3d4ef73 Merge branch 'staging' into fix-survey-view (Brendan Reville) 169026f Merge pull request #9702 from code-dot-org/touch-tunneling-fix (Caley Brock) c863139 Merge pull request #9740 from code-dot-org/prevent-auto-starting-animations (Brad Buchanan) 12ee7ac Merge pull request #9729 from code-dot-org/survey-fixes (Brendan Reville) c3af7a1 Merge branch 'staging' into survey-fixes (Brendan Reville) 98ace8a Teacher dashboard: Fix teacher view default course (Brendan Reville) 564d930 chat bubble tip stroke should default to none rather than white (Elijah Hamovitz) 9fba967 Clean up test syntax from code review (Caley Brock)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Converts the NetSim router log modal dialog to render with React instead of an EJS template, in preparation for new teacher-only log browser features. Uses the new
Dialogcomponent and more Code.org-standard styles than the old log browser. Currently behind an experimentnetsimNewLogBrowser.Here's the new log browser:
And here's the old one, running on this branch with the experiment disabled:
The new browser has the same features and limitations as the old one, but reuses more code (Reactabular.js in particular) and I'll be using its search/filter features soon.
Once we're ready to permanently remove the non-React version we'll be able to tear out a lot of code 😀 and could gradually move most of the business logic in NetSimRouterLogModal.js into NetSimLogBrowser.jsx.