Simulate indentation of the project tree using some divs by ficristo · Pull Request #12047 · adobe/brackets

ficristo · 2016-01-04T12:08:45Z

instead of using combinations of padding/margin.

ficristo · 2016-05-15T13:12:33Z

@petetnt @busykai @nethip
Could you take a look?

itsmunim · 2016-05-18T08:44:08Z

+     * @param {int} depth - The depthness of the current node.
+     * @return {ReactComponent} - The resulting div.
+     */
+    function _createThickness(depth) {


Does this div have defined display as inline-block?

I just saw it is defined in the thickness class, may be we can define it inside the style prop?

ficristo · 2016-05-18T08:56:47Z

@dibosh Thank you to have looked at it.
I think is better to use a class so extension authors can override it.
I use the style only because I need it to be dynamic.
If there is an agreement to use only the style prop is fine by me.

ficristo · 2016-07-23T10:32:18Z

@marcelgerber if you have a bit of time could you take a look?

marcelgerber · 2016-07-23T21:30:42Z

+     * @private
+     *
+     * Create an appropriate div based thickness to indent the tree correctly.
+     *


Nit, but depthness is no word ;)

marcelgerber · 2016-07-23T21:43:49Z

LGTM, but I'd like to have someone else have a look at the React code (even though it looks straighforward) since I don't know nothing about React.

ficristo · 2016-07-24T07:13:38Z

@marcelgerber Thank you.
Maybe @busykai or @petetnt could you take a look?

petetnt · 2016-07-25T21:17:09Z

+                    className: "jstree-icon"
+                }, " "),
+            ];
+            var thickness = _createThickness(this.props.depth);


Nit: Consecutive vars are usually either separated with a dot or new line

petetnt · 2016-07-25T21:25:24Z

Reviewed this, made a few comments 👍

ficristo · 2016-07-29T14:00:38Z

I've updated the PR.
I should note there is still an issue while renaming a file: I would prefer to look at it in a different PR.

petetnt · 2016-07-29T14:36:04Z

@@ -114,35 +114,36 @@ define(function (require, exports, module) {
    /**
     * @private
     *


Unitless value here, does React automatically convert it to px?

ficristo · 2016-07-29T17:48:55Z

@petetnt I tryed to remove the unit and it worked...
Now I looked at the docs and if you don't specify the unit most of the styles are automatically converted to use px.

marcelgerber · 2016-08-18T00:17:21Z

@ficristo @petetnt I suppose this is ready to go?

petetnt · 2016-08-18T06:46:54Z

@marcelgerber LGTM, but has merge conflicts after some recent changes.

ficristo · 2016-08-18T07:00:17Z

I made some changes so to fix the rename case too. So it needs another look.

marcelgerber · 2016-08-18T10:21:47Z

Renaming a nested folder doesn't really work.

ficristo · 2016-08-18T13:54:34Z

@marcelgerber I fixed that issue, but now when you rename a folder the element with the children (ul) move a little. I wasn't able to find the cause, and I think it is a minor issue.

There is a renameBehavior function that should be prevent the directories to collapse, but seems to not work, that's why I've added some more stopPropagation.
I don't know enough of react to really look at this problem.

marcelgerber · 2016-08-18T17:45:51Z

You probably didn't see this comment:
As it turns out in #11717, the indentation (and as such, INDENTATION_WIDTH) should only be 10px, and now that it's merged, that's the behaviour on master, too. I think it looks way cleaner.

marcelgerber · 2016-08-18T18:09:48Z

If you can somehow manage to not apply .jstree-rename-input's top/left rules to the folders, but only to files, we can get rid of the jumping box.

instead of using combinations of padding/margin. Fixes adobe#10574

…iArgs in rename

ficristo · 2016-08-20T12:40:32Z

I noticed another small issue were selecting a folder would result in a blue border.
So I reworked the code for render the folder to be more similar to the one of the files: in the renaming case the html layout is a bit different in the sense the input is not anymore in a link tag.
And then hopefully fix the bouncing behaviour: I applyed a style both to folders and files to which doesn't change anything visibily.

marcelgerber · 2016-08-20T15:49:19Z

LGTM. Going for it.

ficristo · 2016-08-20T15:54:28Z

Thank you @marcelgerber @petetnt @dibosh for review

Update tests after #12047

itsmunim reviewed May 18, 2016
View reviewed changes

marcelgerber reviewed Jul 23, 2016
View reviewed changes

petetnt reviewed Jul 25, 2016
View reviewed changes

petetnt reviewed Jul 29, 2016
View reviewed changes

ficristo force-pushed the fix-folders-tree branch from d865862 to 39416d3 Compare August 18, 2016 06:58

ficristo added 5 commits August 20, 2016 11:15

Simulate indentation of the project tree using some divs

1dda74c

instead of using combinations of padding/margin. Fixes adobe#10574

Review comments

ef80a26

Remove thickness class, add stopPropagation in rename, fix order of l…

de1f63f

…iArgs in rename

fix folder behaviour

80aeead

Rework folder code and fix bouncing effect

dc8cb32

ficristo force-pushed the fix-folders-tree branch from 5969de2 to dc8cb32 Compare August 20, 2016 12:36

marcelgerber merged commit 23f3eb6 into adobe:master Aug 20, 2016

ficristo deleted the fix-folders-tree branch August 20, 2016 16:10

ficristo added a commit to ficristo/brackets that referenced this pull request Aug 21, 2016

Update tests after adobe#12047

1c2a74d

ficristo added this to the Release 1.8 milestone Aug 21, 2016

marcelgerber pushed a commit that referenced this pull request Aug 21, 2016

Merge pull request #12700 from ficristo/fix-filetreeview-tests

9f0825d

Update tests after #12047

ficristo mentioned this pull request Mar 26, 2017

Investigate changing html layout for file tree #13226

Open

This was referenced Aug 30, 2021

[CLOSED] Simulate indentation of the project tree using some divs brackets-archive/bracketsIssues#10378

Open

Investigate changing html layout for file tree brackets-archive/bracketsIssues#13812

Open

Conversation

ficristo commented Jan 4, 2016

Uh oh!

ficristo commented May 15, 2016

Uh oh!

itsmunim May 18, 2016

Choose a reason for hiding this comment

Uh oh!

itsmunim May 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ficristo commented May 18, 2016

Uh oh!

ficristo commented Jul 23, 2016

Uh oh!

marcelgerber Jul 23, 2016

Choose a reason for hiding this comment

Uh oh!

marcelgerber commented Jul 23, 2016

Uh oh!

ficristo commented Jul 24, 2016

Uh oh!

petetnt Jul 25, 2016

Choose a reason for hiding this comment

Uh oh!

petetnt commented Jul 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ficristo commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petetnt Jul 29, 2016

Choose a reason for hiding this comment

Uh oh!

ficristo commented Jul 29, 2016

Uh oh!

marcelgerber commented Aug 18, 2016

Uh oh!

petetnt commented Aug 18, 2016

Uh oh!

ficristo commented Aug 18, 2016

Uh oh!

marcelgerber commented Aug 18, 2016

Uh oh!

ficristo commented Aug 18, 2016

Uh oh!

marcelgerber commented Aug 18, 2016

Uh oh!

marcelgerber commented Aug 18, 2016

Uh oh!

ficristo commented Aug 20, 2016

Uh oh!

marcelgerber commented Aug 20, 2016

Uh oh!

ficristo commented Aug 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

itsmunim May 18, 2016 •

edited

Loading

petetnt commented Jul 25, 2016 •

edited

Loading

ficristo commented Jul 29, 2016 •

edited

Loading