feat($urlParams): add params for isInProjectView + initialPath #49

faceyspacey · 2017-07-11T21:46:12Z

These options update the capabilities of <ShareView /> and on app load the query params are forwarded on to<DirectoryEntry />, <Preview />, etc:

the Single file entry script parameter (referenced in code as isInProjectView) replaces current currentModule == null checks, but falls back on the old way if false, to insure there is a current module
initialPath is forwarded on to the iframe
there's also now a cancel link above the module selection in case you want to back out of selecting a custom entry file.
also the default value for isInProjectView within the ShareView modal inherits from the current fork :)

faceyspacey · 2017-07-11T22:33:30Z

ps. I haven't tested via the /embed URLs cuz i dont know how to get those working yet. Let me know how and I will.

CompuIves · 2017-07-11T23:33:41Z

Wow this looks great!! I'll do a thorough review tomorrow morning, I want to rush to bed first 😅

faceyspacey · 2017-07-11T23:36:10Z

Keep killing it my friend! Ur a soldier. Ur on the verge of making ur entire career, just so u know. Talk tomorrow.

CompuIves

This is really great! I have some minor suggestions, but overall this is very clear and well written.

CompuIves · 2017-07-12T10:55:26Z

src/app/utils/url-generator.js

@@ -45,11 +45,13 @@ export const embedUrl = (sandbox: Sandbox) => {
 };

 export const frameUrl = (append: string = '') => {
+  const path = append.indexOf('/') === 0 ? append.substr(1) : append;


What's the reason for adding this check?

Yes perhaps it could be more consistently handled by not getting that initial slash in the first place. But it does and I figured we are better safe than sorry. It's the slash the user enters in the initial path field. So if u want I can strip it earlier.

No this is great! Let's keep it 😄

CompuIves · 2017-07-12T10:56:05Z

src/common/url.js

@@ -7,14 +7,19 @@ export const getSandboxOptions = (url: string) => {
    result.currentModule = moduleMatch[3];
  }

+  const initialPathMatch = url.match(/(\?|&)(initialPath)=([^&]+)/);


This is great! Nice find 😄

Your app is well organized

CompuIves · 2017-07-12T10:57:21Z

src/embed/App.js

-        sandbox: response.data,
-        currentModule:
-          this.state.currentModule ||
+      const currentModule = this.state.currentModule ||
          response.data.modules.find(
            m => m.title === 'index.js' && m.directoryShortid == null,


Hmm, not part of the review, but maybe we could change this to findMainModule from the modules selector. Seems like I did a quick hack here.

Thought so myself but didn't wanna touch it

CompuIves · 2017-07-12T10:58:18Z

src/app/store/entities/sandboxes/modules/selectors.js

+  mainModule: Module
+): Module =>
+  modules.find(m => m.id === currentModuleId) ||
+  modules.find(m => m.shortid === currentModuleId) || // deep-links requires this


Good point, also great that you commented the second use case.

CompuIves · 2017-07-12T11:01:10Z

src/app/pages/Sandbox/Editor/Content/Header/ShareView.js

@@ -296,11 +343,25 @@ class ShareView extends React.PureComponent {
                      setValue={this.setHideNavigation}
                    />
                    <PaddedPreference
+                      title="Single file entry script"


Maybe we should make this title some more clear, something like "Show selected module". Still not very descriptive, but maybe a bit better. It's very hard to describe this in one line.

We could also change this preference to the actual switch button from the preview, like we do for the view modes. Then the ui explains the functionality.

I was gonna do that. But I figured this is less limiting. It doesn't require what ur lookin at to he what u give other people.

Basically the shareview needs a big question mark in a top corner and a backside with descriptions of everything. Coming up with a better name for this one field won't solve the problem here.

Personally I would get rid of individual files being able to run themselves. Am I missing something? How can u even do that with webpack. I mean I don't see it being a popular use case for CSB to be used for more than one entry point. Is that what u r getting at? Let the user create infinite entry points. U can only support 60 files. Maybe one day, but now isn't the time.

In short get rid of this switch in the top right if the preview and what I made. Et voila. No better naming, faq backside is less important. No potentially showing beige and pink pages of death.

Nobody can figure out what that switch in the top right of the preview does. That's my hunch. I couldn't as u know.

Less is more. Kill it. I can see ur codebase is becoming burdensome to u by the number of linting and flow errors. U have hit the inflection point where it's not gonna be fun without first cleaning that up. And more importantly removing some features. It's really astounding how much u got accomplished. But u took on a lot. Especially while going to school. Id make a list of stuff u can get rid of and just be strong and do it. So u don't gotta paste those smileys with tears on it anymore. I'm not joking. This school year will be a nightmare for u as this gets popular. We both know this. Drop out or find a way to reduce ur strategy and codebase. Or get help :).

we should get rid of the file selection box too. just inherit once again.

that also means 3 buttons for "Default View" could go too. ..i dont know how i feel about it. im sure ur more attached. i do find them useful. but only because I know what they mean. Truthfully, those 3 buttons i couldnt guess what they did until i clicked them eventually. And still it was confusing. I know it's supposed to be obvious, but for some reason it's not. They kinda just sit and look pretty.

It's key to ur brand though, so i wouldnt remove those buttons. I just think getting this embed right will be important. 1, it's less to manage. 2, if it's easy, it means more sharing, which means more traffic.

So here's what i would do: keep the 3 switches on the full editor, as it's good for the brand, and it is useful, and i was just dumb and probably in the minority for it not making sense at first.

However, for the embed, dont offer the 3 default view switches. Instead just always default to split screen. Period. That's your brand there--u want people to see CODE + AN APP when they see ur embed. In addition, if ur not in split mode, the screen is too wide for the alloted space. It only makes sense in split mode. Nobody is making super wide screen apps for example--it looks bad. Split screen looks great. Default to split screen, remove those buttons.

I'd also say: forget the HIDE NAVIGATION BAR. U want that--even if many sandboxes just use /. The reason is it's a visual que that the user is looking at a demo web browser. Plus, with RFR, I promise people will start using the URL more. It makes it stupid simple to URL-ize any redux app. ..But also u should promote using URLs since it's good for everyone. SPAs are dead. They are no good. The URL makes it nice to see demos in a few contexts and let users go back and forth between them. So it's good to have ur sandbox creators thinking about creating different URLs for their app, rather than having it solely be interactive.

Anyway, that's the case for always keeping the address bar. The case against it is 48 pixels in height lost. One more case for it: the address bar makes the preview vertically even with the code editor.

Forget the font-size too.

All this stuff is friction. Here's the concept: YOU ALREADY HAD A USER WILLING TO SHARE! NOW THEY SEE THIS, THEY ARE OVERLOADED WITH DECISIONS AND A LARGE PERCENTAGE NOW DO NOT SHARE.

The only thing i dont know about is the "auto resize" feature for medium. What is that?

Lastly, here's what I would do:

That's a shitty example, but the idea is find somewhere else to move that shit, remove the ShareView modal, and just put the damn URL to share there. It already automatically embeds on Medium.

For the embed/iframe URL, instead just write code that detects ur being run in an iframe. So now u just got rid of 2 of the 3 URL fields and the iframe textarea (though i understand u want ur border radius, and ideally we have those iframe sandbox values...But anyway, ur left with markdown and html for the button and maybe the iframe. Put that as a secondary share option for the hyper-motivated--but the overall idea is make the main URL just one URL and stupid simple and ever-present to copy paste. That's why im suggesting to put it in the nav bar. But what's better is: a footer that contains just this.

so what do we do now?

we let the above simplification sink in

remove the new switch and the switch from the address bar (that means removing the ability for the user to change isInProjectView completely--i think that will greatly simplify things both for u and for users; i understand u had the idea of using this as a "gallery" where each file is something different to look at, but that's not how webpack is being used--has anyone even done that? ...i checked every demo on the homepage and everyone is in isInProjectView--get rid of the option!!)

i'll try to figure out how to not put the moduleId in the URL if it's not manually selected

i'll make the file directory in ShareView auto-select the same file open in the editor. I'll remove the link to cancel it--now it's expected u always choose a default file! And now it causes no problems cuz the embed always evaluates the entry script

i'll keep all the other options for now. at the end of the day, u dont have the time to change all that around anyway and it's solid. so forget my thoughts and leave it, but let's simplify the isInprojectView + file selection stuff completely. If it's "the way it works" that changes everything and makes everyone's lives simpler.

That's it. What do u think?

and honestly, i'd just put the moduleId in the URL. And here's why: if u choose the direction i've laid out, you're now trying to get the user to see that they can control what module is displayed--that's a core feature. So u want them to see it in the URL now. And here's what makes it more obvious: the id is the file name, not the real id. That way they can manually change it on their own, etc. It's the full url-encoded file path.

So we should transition to file paths later (should u approve), and then now just always keep the module Id in the URL as that would be the direction we're heading.

I can do this for u in the stage 2.

and actually it should be just like github (not URL-encoded):

https://github.com/faceyspacey/redux-first-router/blob/master/src/connectRoutes.js

see how ur app's file structure becomes a standard part of the URL. THAT IS THE CORRECT WAY!

if u called history.push every time the user changed files, ur copy/paste field would be the address bar! ...that's the real goal here.

CompuIves · 2017-07-12T11:01:49Z

src/app/pages/Sandbox/Editor/Content/Header/ShareView.js

+      defaultModule: null,
+      autoResize: false,
+      hideNavigation: false,
+      isInProjectView: props.sandbox.isInProjectView, // thats why i moved this to the constructor :)


Interesting, taking over the sandbox project view. I like it!

CompuIves · 2017-07-12T11:03:25Z

src/app/pages/Sandbox/Editor/Content/Header/ShareView.js

@@ -169,6 +193,7 @@ class ShareView extends React.PureComponent {
  setMixedView = () => this.setState({ showEditor: true, showPreview: true });

  setDefaultModule = id => this.setState({ defaultModule: id });
+  clearDefaultModule = () => this.setState({ defaultModule: null });


I think that it's more clear for the user if we select the main module by default, and don't append module=blah to the url if the main module is selected. What do you think?

I agree. Cleaner URLs are better. Observant developers will pay attention to the query params. But I didn't know how to do it with proper guarantees. I couldn't be sure that with no id selected it worked since I'm new to the codebase. I wasn't sure what else u were doing.

Anyway u made this decision early on, it's probably easier for me now.

I think you could add a check to the url generator like if (defaultModule && defaultModule !== mainModule.shortid) { /* add the url thingie */ }.

faceyspacey · 2017-07-12T17:24:36Z

The final summary is:

always in project view and remove UI to toggle it in embed and preview
by default the file directory in share view auto selects your current file when u first open share view
keep the module id in the URL now since its core (in the future we go to github style full paths that change as u select different files in the editor file directory)
remove cancel module selection feature I added
remove entry script switch I added

CompuIves · 2017-07-12T17:43:32Z

Unfortunately we cannot go with settings and module in the url, I tried this and it doesn't work since iframes share the same history with the parent frame. This means that every push to the parent frame will result in a push in history in the iframe, which in turn means that the back + next buttons will start to disfunction. It's a real limitation for the editor page. But now come to think of it, maybe it would work if we'd use history.replace?

I agree that the project view and the module view button confuses. It's funny, because I added the project view option as almost last thing before releasing. I'm not looking to simulate the webpack environment, I just want compatibility. If we'd simulate the webpack environment 100% we'd have the same limitations and we wouldn't be able to tailor the editor for great UX.

I have plans for the preview pane with custom renderers for specific types and libraries, like documentation view, redux devtools, markdown preview. I don't think that the mentioned switch is the best way to handle this, but until I know a better way I want it to stay. There are users that find this feature important, I saw that users style components using module view instead of project view (it will render the default exported component if DOM doesn't change). The settings is also not sandbox specific, but is only session specific, it will always default to project view.

(in the future we go to gothic style full paths that change as u select different files in the editor file directory)

This was already built in CodeSandbox, but I had to remove it for the first mentioned reason.

I agree that we need to simplify the ShareView a lot, I don't agree that we should remove it altogether. We should rather give the user a few options, not many, and a button to show advanced settings. I added the auto resize, remove navigation and view modes on request for other users, they all had their valid use cases and changing how the initial state of an application is isn't that hard to add.

I don't think I've reached my max complexity yet, far from it. Of course there'll need to be rewrites, but that's the idea of iteration driven development. I'd say that the client is also really small compared to the other things that encompass CodeSandbox. CodeSandbox now has:

Phoenix Server (Elixir)
Postgres Server
Redis Server
NGINX Server
Homepage (Plain HTML + CSS)
Client (React)
Sandbox (vanilla JS)
Git Extractor (Node)
CLI
Algolia Integration
Stripe Integration
Dependency Packager (Node)
MongoDB server (used by packager dll service)

The client is only number 6 of that list.

Also you're right, I definitely need to fix the lint + flow errors. I need to put a night to work on this 😅 .

by default the file directory in share view auto selects your current file

I think this would be more confusing, if I'd want to share a sandbox I wouldn't think that my opened file will be opened in the embed by default.

faceyspacey · 2017-07-12T18:01:18Z

ur redis server/db is more complex than the this client?? I'd think the client + phoenix server would be competing for spot #2. I really hope the client isn't smaller and less complex than all those other things. It's already huge. I personally find server side development far easier these days--since debugging is painless and the execution path is basically sequential, and since expectations for the client experience is so high in 2017.

Regarding:

I'm not looking to simulate the webpack environment, I just want compatibility. If we'd simulate the webpack environment 100% we'd have the same limitations and we wouldn't be able to tailor the editor for great UX.

I'm not referring to a sweeping change of simulating the whole environment. I think a few core expectations from how webpack works is very important for the growth of CSB at this stage. I think module view is a mistake at this stage, since most your early adopters expect a single entry script. The thing I've learned is that certain features make sense at different times. The module view way to view files as a gallery will make sense when u already have full traction and aren't trying to convince/please your first audience. I would make this as pleasing as possible for your first audience. And without a doubt that does not include module view. It messed me up and confused me and I'm sure I'm not the only one. ...Forget the one user that accidentally used module view. He can use another package like react-router to correctly create pages should he really need.

I have plans for the preview pane with custom renderers for specific types and libraries, like documentation view, redux devtools, markdown preview.

..Wait until you get to those plans. Make this perfect for the 80% use case now. My 2 cents.

I'll get on what u want. Let me know if anything i just said changed ur mind about hiding module view for the time being.

CompuIves · 2017-07-12T18:10:14Z

No, I mean that every service combined is more complex than the editor. The client is still most complex, definitely. Like you said the server is second place, packager shares that spot.

I don't see how the current module view is blocking for anyone. I totally understand that it's confusing if module view gets automatically activated if you select a module to preview, but I thought that this PR was aiming to fix that by making it an option that's off by default. The current module view is far more powerful than a react-router page, it can actually analyze the module that's exported and show specialized information. It's like a magnifying glass, because you can read and alter the export. It's not used for as a drop in replacement for routing.

faceyspacey · 2017-07-12T18:15:03Z

i understand that it's powerful. now that i know how it works i love it. and i get all the automation u put behind it to just grab the default export and anything else related. LOVE IT. get it.

But for tinkerers (which we all are since we are developers) we are 60% likely to do what I did and toggle that switch to see what it does and end up in a bad place. I did that and thought this stuff was just broken and not fully ready for prime time. I didn't even know what i did since i had tinkered with so many things, so i struggled to put it back in a solid state. I would have given up had I not had my project which I intuitively knew would shine within ur product if i could get it working. Most your users aren't immediately hungry to create a sandbox from scratch--their tinkering here will mess them up.

Hide it for now brother. I know how it feels to put a ton of work into something and realize u dont currently need it.

Either way, i dont really care now that i know how to get what i need. Let me know once again if u change ur mind. I'll be working on this in an hour.

faceyspacey · 2017-07-12T18:21:44Z

one other perspective:

basically its too much to think about with a new product. it would be one thing if u already had many power-users, and the addition was logical to them. and for new users, there was a forum where they explained it to them.

the growth curve of a product is a very interesting thing. essentially ur worse off having every feature in the book than you are incrementally adding it. you're telling a story this way, and your users need less help cuz they intuitively understand the evolution. but u miss all that by being the fast developer you are and adding everything at once.

excite your users by incrementally adding things.

last perspective: the initial stage of the product is the most important. think about webpackbin--nobody uses that. here's the thing: i built my sandbox for webpackbin LONG LONG ago. When I realized they didnt have directories I stopped. i just finally got around to releasing it. it was perfect timing in fact. the stars aligned and u finished ur product right before i was truly ready to launch my bin/sandbox.

so if u hit the nail on the head and delight now it will be the catalyst to your growth. u dont want any users slipping through the cracks because they didn't get it. so if u can be disciplined and nail it for that 80% use case, ur delighting the highest percentage of users, and u will go on to stardom unlike webpackbin. Yes, there are many reasons how CSB is better. But the general principle is true.

If ur gonna do "gallery mode" put it in the left column in an accordion menu that someone who really wants the feature can find. end of story.

faceyspacey · 2017-07-12T18:23:54Z

but that all said, perhaps today it's best to change the least things. that's another principle of process i adhere to--marinate on decisions like this. i'll keep the switch, and do as u want. And I'll address this point:

I think this would be more confusing, if I'd want to share a sandbox I wouldn't think that my opened file will be opened in the embed by default.

faceyspacey · 2017-07-12T18:27:12Z

"gallery mode" or even better: "file eval" ...u know like those IDE plugins that eval files. I think that's the best way to market/position this feature. That way it doesn't take away from the most common expectation of single file entry scripts.

then u put a checkbox in the Files accordion row that says "[x] file eval mode"

faceyspacey · 2017-07-12T18:30:36Z

thats one option. it may or may not be it. but what it does accomplish is "context." By putting the option here and naming it so, it's more familiar to developers what it might do. Whereas in the top right by the address bar makes zero sense. Perhaps placement is the primary problem. By the address bar, u would think it has to do with the URL. But it doesnt. Here the context makes it far clearer.

faceyspacey · 2017-07-12T18:31:51Z

add a tooltip containing one sentence to "eval mode" and done. everybody knows what it does.

"Eval mode allows you to re-evaluate each file as you click it, instead of your main entry script. It's great for galleries."

faceyspacey · 2017-07-12T18:36:59Z

ITs called just "file eval"

faceyspacey · 2017-07-12T18:42:07Z

Then the name of my share view switch is also "file eval" and now everything is clear

faceyspacey · 2017-07-12T20:59:25Z

ok i just started. would be nice to hear if u like the idea. Im guess u might--cuz it's better. Not saying there isnt even better. But hey, one step at a time. Here's a screenshot of the actual app:

…ar, re-incarnated it as isFileEval in the Files WorkspaceItem

faceyspacey · 2017-07-12T21:56:07Z

Ok, the photo is now real. pull in the changes. Do you use:

https://hub.github.com/

by the way?

you can do just git checkout https://github.com/CompuIves/codesandbox-client/pull/49 from the command line.

CompuIves · 2017-07-12T22:36:53Z

I'm using something else for PRs, but comes down to the same thing. ☺️

Let me think about the switch for a bit. I currently feel like the switch would be better in the context of the preview, since it alters what the preview shows. I don't think that file eval is a correct way, since in reality we evaluate all files, it's just which file gets evaluated.

I still currently think that it's better to just change the default behavior of the embed for now. I have a bunch of plans for this view in the sandbox rewrite I'm planning and don't want the user to switch twice. A less intrusive change would be to show a different message on the yellow screen, explaining why it's not rendering and what the function does.

But like I said, I'll sleep a night over it and see if I changed my mind ☺️.

CompuIves · 2017-07-12T22:37:19Z

My MacBook died and I forgot my charger so I'm powerless right now 🙁

faceyspacey · 2017-07-12T23:21:47Z

it doesn't look like im best suited for opening the directories, not without more discussion between us. There's a lot going on there. It also doesn't look like you have an API to recursively open directories based on the directory short ID of a possibly deeply nested file. I'm sure you'll have a certain way you want that done--so i'll leave the rest in your hands.

I passed currentDirectoryShortid of the current module to DirectoryEntry within Workspace/Files/index.js. That's where I left off.

Here's exactly where I left off for your reference:

CompuIves · 2017-07-13T14:58:02Z

Okay good! I can add the automatic directory opening based on module id, you don't need to pass currentDirectoryShortid to the DirectoryEntry in this case. There are three things left before merge:

I'd remove the file eval switch from the workspace, the workspace isn't intended for altering the preview configuration (Preferences are also there in the wrong place, they will move to a modal). It's rather for Sandbox settings only. I think 2 switches doing the same thing are confusing too.
Let's make project view default at all times, also when a module is set. This will remove all confusion. Only when the user actively clicks on the switch it will show Current Module View, if the yellow screen appears it's at least following an interaction. Much clearer for the user.
We can drop the cancel button from the ShareView, we can just check if the selected module is main module and not add it to the url. It's simpler for the url and more intuitive. I can add this if you want to.

And for after this PR has been merged I'll make points for this:

Make the yellow screen description more clear for the user, show options.
Move Preferences to a modal.
Move all advanced features behind an 'Show advanced' button in ShareView.

And for after pricing and search:

Redo the rendering for custom renderers etc, this is necessary regardless because of template support.

CompuIves · 2017-07-13T14:59:08Z

src/common/url.js

@@ -24,6 +29,7 @@ export const getSandboxOptions = (url: string) => {
  }

  result.hideNavigation = url.includes('hidenavigation=1');
+  result.isInProjectView = !url.includes('isFileEval=1');


I'd rename this to moduleview=1.

It should just go back to isInProjectView to stay consistent and introduce less names. I only changed it since the feature split into 2 concepts. We can use =0 so it only shows in module view.

CompuIves · 2017-07-13T14:59:42Z

src/app/pages/Sandbox/Editor/Workspace/Files/index.js

@@ -62,8 +62,8 @@ class Files extends React.PureComponent {
        modules={sortBy(modules, 'title')}
        directories={sortBy(directories, 'title')}
        isInProjectView={sandbox.isInProjectView}
-        // $FlowIssue
-        currentModuleId={currentModule || mainModule.id}
+        currentModuleId={currentModule.id}


This is nice, much cleaner.

CompuIves · 2017-07-13T15:00:47Z

src/app/components/sandbox/Preview/index.js

+    // we need a value that doesn't change when receiving `initialPath`
+    // from the query params, or the iframe will continue to be re-rendered
+    // when the user navigates the iframe app, which shows the loading screen
+    this.initialPath = this.state.urlInAddressBar


Do you mean with this navigating inside the iframe, or manually entering the url?

I meant navigating when the app uses history.push like my sandbox. It was refreshing because the URL kept changing. It kept changing because no longer was passed just the base URL.

So I fixed it by making sure it got the URL, but only once on mount. That way it can receive initialPath from query params. It doesn't need to think about it anymore after the first render. Put back urlInAddressBar and try clicking links in my codesandbox to see.

CompuIves · 2017-07-13T15:02:41Z

src/app/components/sandbox/Preview/index.js

@@ -158,7 +166,8 @@ export default class Preview extends React.PureComponent {
    const element = document.getElementById('sandbox');

    if (element) {
-      element.contentWindow.postMessage(message, frameUrl());
+      const { urlInAddressBar: url } = this.state
+      element.contentWindow.postMessage(message, frameUrl(url));


I don't think this needs to have the url, it's a check for targetOrigin (https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage).

Yes I think so too. Undo it

faceyspacey · 2017-07-13T17:26:00Z

FYI I originally deleted the switch next to the address bar. I only put it back so you could choose. For me it was finally clear how this sandbox worked, though I still missed the sexiness of your original switch. And by the way the branding of its colors and uniqueness are the only reason I say to keep it.

Same with having the cancel link in the share view. Being able to select a module and then have no way to unselect it was disconcerting. Especially at a time when I was yet to understand the difference between the 2 project/module view modes. The thing is everything is subjective. Something intuitive to one person isn't always to another and vice versa. So ur at a point where it's so obvious to U since u have been with it for so long that ur suspectible to forgetting what it's like for new users. Using additional message--such as in the iframe when it breaks--is never the ideal solution. its only a good enough temporary solution, but that's fine sometimes.

I'm on vacation until Monday. Won't be doing any coding. I can answer questions tho from my phone as I'm doin now. Why don't u just wrap it up brother since u know how u want it.

CompuIves · 2017-07-13T18:09:02Z

So ur at a point where it's so obvious to U since u have been with it for so long that ur suspectible to forgetting what it's like for new users. Using additional message--such as in the iframe when it breaks--is never the ideal solution.

The thing is that I completely understand your frustration. I also understand that the disability to unselect is frustrating, that's why I propose to always select a file, that makes it so much clearer for the user. Just select index.js by default.

Also implicitly changing view is also confusing, as most users don't know what it means. Only allowing this explicitly will fix that too.

I'll wrap the PR up.

CompuIves · 2017-07-16T21:31:44Z

Nice! Could you update the .all-contributorsrc and README.md by checkout out the new one and running npm run add again? The we're all set to merge it in!

CompuIves · 2017-07-16T21:32:22Z

src/app/pages/Sandbox/Editor/Workspace/Files/DirectoryEntry/DirectoryChildren.js

@@ -72,7 +72,7 @@ export default class DirectoryChildren extends React.PureComponent {
        {modules.filter(x => x.directoryShortid === parentShortid).map(m => {
          const isActive = m.id === currentModuleId;
          const mainModule = isMainModule(m);
-
+          console.log('is ACTIVE', isActive, isActive && m)


Oh one last thing, could you remove this line?

CompuIves · 2017-07-16T21:44:40Z

Great, I just saw that the snapshots for url-params need to be updated. That's finally really the last thing that needs to be done haha.

faceyspacey · 2017-07-16T21:47:08Z

i just ran npm test -- -u. hopefully that's what you wanted.

CompuIves · 2017-07-16T21:47:57Z

That's it! Okay I'll merge it in after the tests give the green light. Thanks a bunch for this contribution!

faceyspacey · 2017-07-16T21:50:05Z

not a problem, thanks for taking the needs of your "customers" so seriously.

faceyspacey added 4 commits July 11, 2017 14:39

feat($urlParams): add params for isInProjectView + initialPath

fb181c1

fix($flow): addd isInprojectView + initialPath to Sandbox flow type

1704c37

Add @faceyspacey as a contributor

7ca8118

Add @faceyspacey as a contributor

9b8f13e

CompuIves reviewed Jul 12, 2017

View reviewed changes

feat($fileEavl): removed toggle isInProjectView switch from address b…

5c64bfa

…ar, re-incarnated it as isFileEval in the Files WorkspaceItem

faceyspacey added 3 commits July 12, 2017 15:01

fix($deleteLogging): remove console.logs

ba319dc

fix($checked): switch checkbox value to checked attribute

ba900a2

fix($logging): remove logging

ef70681

faceyspacey added 2 commits July 12, 2017 15:44

fix($frameUrl): insure iframe doesnt re-render when its URL changes

d571d52

bring navigator project/module view switch back

0a11495

start($currentDirectoryShortid)

89ce1ce

CompuIves reviewed Jul 13, 2017

View reviewed changes

CompuIves mentioned this pull request Jul 16, 2017

Allow directly linking to a URL #57

Closed

Ives van Hoorne added 7 commits July 16, 2017 14:26

Rename path variables

889b5b9

Remove file eval switch

27e0ae1

Remove directoryShortId from Files component

f07d0d0

Select index.js defaultModule by default in ShareView

9a64f62

Change current module view wording

f9bc699

Make embed work with URL parameters

1d6fb21

Auto open directory of current module

9089f68

CompuIves reviewed Jul 16, 2017

View reviewed changes

faceyspacey added 3 commits July 16, 2017 14:36

Merge branch 'master' into url-params

cd0d70b

fix($logging): remove logging

6d28e42

Update @faceyspacey as a contributor

8499746

chore($tests): update test snapshots

7e0526b

CompuIves merged commit 66e6a71 into codesandbox:master Jul 16, 2017

faceyspacey mentioned this pull request Sep 7, 2017

Optional path parameters? faceyspacey/redux-first-router#83

Closed

snyk-bot mentioned this pull request Jul 5, 2020

[Snyk] Upgrade circular-json from 0.4.0 to 0.5.9 sitedata/codesandbox-client#3

Open

feat($urlParams): add params for isInProjectView + initialPath #49

feat($urlParams): add params for isInProjectView + initialPath #49

Conversation

faceyspacey commented Jul 11, 2017 • edited Loading

faceyspacey commented Jul 11, 2017

CompuIves commented Jul 11, 2017 • edited Loading

faceyspacey commented Jul 11, 2017

CompuIves left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faceyspacey Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faceyspacey commented Jul 12, 2017 • edited Loading

CompuIves commented Jul 12, 2017 • edited Loading

faceyspacey commented Jul 12, 2017 • edited Loading

CompuIves commented Jul 12, 2017

faceyspacey commented Jul 12, 2017 • edited Loading

faceyspacey commented Jul 12, 2017 • edited Loading

faceyspacey commented Jul 12, 2017 • edited Loading

faceyspacey commented Jul 12, 2017

faceyspacey commented Jul 12, 2017

faceyspacey commented Jul 12, 2017 • edited Loading

faceyspacey commented Jul 12, 2017

faceyspacey commented Jul 12, 2017

faceyspacey commented Jul 12, 2017

faceyspacey commented Jul 12, 2017

CompuIves commented Jul 12, 2017

CompuIves commented Jul 12, 2017

faceyspacey commented Jul 12, 2017

CompuIves commented Jul 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faceyspacey commented Jul 13, 2017

CompuIves commented Jul 13, 2017 • edited Loading

CompuIves commented Jul 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CompuIves commented Jul 16, 2017

faceyspacey commented Jul 16, 2017

CompuIves commented Jul 16, 2017

faceyspacey commented Jul 16, 2017

faceyspacey commented Jul 11, 2017 •

edited

Loading

CompuIves commented Jul 11, 2017 •

edited

Loading

faceyspacey Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

CompuIves commented Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

faceyspacey commented Jul 12, 2017 •

edited

Loading

CompuIves commented Jul 13, 2017 •

edited

Loading