[admin] added full screen mode to advanced json editor #43

gastonche · 2017-03-11T09:25:55Z

PR pertaining to issue #41 .

coveralls · 2017-03-11T09:28:30Z

Coverage remained the same at 99.877% when pulling 3b41207 on gastonche:fullscreen_editor into 3a7287d on openwisp:master.

gastonche · 2017-03-12T10:42:47Z

@nemesisdesign I submitted the PR already and would like you to review it so I can correct any issues you may find soon.

nemesifier · 2017-03-13T14:09:17Z

Testing now.

nemesifier · 2017-03-13T14:33:46Z

I would say we are halfway through it. In order for this to be really full screen, a few slightly more complex things should be implemented:

the height of the advanced editor should adapt to the height of the window, eg: div.height($(window).height())
the height of the advanced editor should change if the window is resized (bind to resize event of window object)
the main scrollbars of the body element must be disabled (set overflow: hidden)
when back to normal mode, the height of the advanced editor must be cleaned up, the overflow: hidden directive also must be cleaned up and the resize event must be unloaded

There are also a few graphical improvements I want to suggest:

in full screen mode, the help text "configuration in NetJSON DeviceConfiguration format" and its container (with white background) can be hidden so that the editor can become prevalent

gastonche · 2017-03-13T14:46:48Z

Okay, I'll update and submit a PR by night. @nemesisdesign

nemesifier · 2017-03-13T14:49:07Z

@gastonche you can update this PR by amending your commit (git --amend) and using git push -f to your own development branch

gastonche · 2017-03-13T14:52:16Z

Okay, thanks for the tip @nemesisdesign

gastonche · 2017-03-14T00:41:40Z

@nemesisdesign I have effected all of the changes you specified. But given that the editor in full screen mode has to hide all other elements, is it sufficient that i have enabled the use of the ESC key as a way to exit full screen mode or should I also add some more controls the the top of the screen hanging over the editor to enable the user exit full screen mode by clicking?

nemesifier · 2017-03-14T09:20:32Z

@gastonche I don't know because I haven't seen the result. Don't be afraid to push your work even if unfinished. Push it, I'll test it and come back to you with some useful suggestions.

coveralls · 2017-03-14T11:06:25Z

Coverage remained the same at 99.877% when pulling 6a0b583 on gastonche:fullscreen_editor into 3a7287d on openwisp:master.

coveralls · 2017-03-16T08:48:39Z

Coverage remained the same at 99.877% when pulling 06ef70e on gastonche:fullscreen_editor into 3a7287d on openwisp:master.

nemesifier

@gastonche great! Much better.

Now, I think the "powered by ace" link takes very important space that we need. Therefore, if you could find a way to hide/delete that element we could put there the button to go back to normal mode.

Do you think it would be possible to move the link to the netjsonconfig documentation in the blue bar on the editor? Or can you think of another way to show that link in a pleasant way?

nemesifier · 2017-03-16T10:14:06Z

django_netjsonconfig/static/django-netjsonconfig/js/widget.js

+                    toggleFullScreen();
+                }
+            });
+        }else{


for consistency with the rest of the code, better to have

} else {

nemesifier · 2017-03-16T10:14:14Z

django_netjsonconfig/static/django-netjsonconfig/js/widget.js

+            inFullScreenMode = false;
+            document.getElementById('advanced_editor').scrollIntoView(true);
+        }
+


delete blank line

coveralls · 2017-03-16T11:56:37Z

Coverage remained the same at 99.877% when pulling 7ef3b28 on gastonche:fullscreen_editor into 3a7287d on openwisp:master.

nemesifier · 2017-03-16T16:06:38Z

Try to squash the 4 commits into one by using git rebase -i HEAD~4

gastonche · 2017-03-16T16:08:09Z

okay, I'll do that, I just wanted you to test that before i finally did

…

On 16 March 2017 at 17:06, Federico Capoano ***@***.***> wrote: Try to squash the 4 commits into one by using git rebase -i HEAD~4 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALaC4KO_KedLT8ua2BA2yQ2Lxvc-oNA3ks5rmV4PgaJpZM4MaJ00> .

nemesifier · 2017-03-16T16:08:42Z

I will test now ;-)

coveralls · 2017-03-16T16:18:18Z

Coverage remained the same at 99.877% when pulling f64bef3 on gastonche:fullscreen_editor into 4278595 on openwisp:master.

coveralls · 2017-03-16T16:18:18Z

Coverage remained the same at 99.877% when pulling f64bef3 on gastonche:fullscreen_editor into 4278595 on openwisp:master.

nemesifier · 2017-03-21T10:28:03Z

@gastonche always ping me when you do changes, we are getting a lot of work and sometimes I may miss some details. Don't be afraid to ping me once in a while.

nemesifier

@gastonche I've tested the changes. We are almost there! I could do these change myself but I'm very busy and I'd like to have your help in finishing this.

change the style of the color of Want learn to use the advanced mode? Consult the netjsonconfig documentation. to white and set the link to netjsonconfig documentation to bold
I think we can safely provide only fullscreen, most users will use this mode and it will make things easier for maintainers, so exit full screen could become "exit advanced mode"

gastonche · 2017-03-24T11:42:29Z

Okay, I have been offline for some days now, but i'll fix that this evening. Sorry about the delay

…

On 22 March 2017 at 13:59, Federico Capoano ***@***.***> wrote: ***@***.**** requested changes on this pull request. @gastonche <https://github.com/gastonche> I've tested the changes. We are almost there! I could do these change myself but I'm very busy and I'd like to have your help in finishing this. - change the style of the color of Want learn to use the advanced mode? Consult the netjsonconfig documentation. to white and set the link to netjsonconfig documentation to bold - I think we can safely provide only fullscreen, most users will use this mode and it will make things easir for maintainers, so exit full screen could become "exit advanced mode" — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALaC4Dn5B1xLsHyWjMeOFZjWmQlT32Dbks5roRtGgaJpZM4MaJ00> .

coveralls · 2017-03-24T14:27:41Z

Coverage remained the same at 99.877% when pulling 8d3ce6e on gastonche:fullscreen_editor into 4278595 on openwisp:master.

gastonche · 2017-03-24T14:30:35Z

@nemesisdesign I think we are Good to Go, I have done the required fixes and updated the PR

gastonche · 2017-03-24T14:31:38Z

@nemesisdesign I think we are good to go. I have done all the requested changes for this PR and updated the PR as requested.

nemesifier · 2017-03-24T16:43:33Z

@gastonche excellent. Very good job. I am merging but I will do a few CSS changes myself and I would like you to take a look at them so that in the future you will be able to do this kind of things without my intervention.

nemesifier · 2017-03-24T17:36:58Z

@gastonche I added some improvements 0dc39c4 and created issue #45 after testing the patch, there are few issues that will have to be resolved before it can be released

gastonche · 2017-03-27T11:33:11Z

Okay, let me refresh my fork and check out the changes you made

…

On 24 March 2017 at 18:36, Federico Capoano ***@***.***> wrote: @gastonche <https://github.com/gastonche> I added some improvements 0dc39c4 <0dc39c4> and created issue #45 <#45> after testing the patch, there are few issues that will have to be resolved before it can be released — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALaC4NCsECx2a7LHh7kqkf1uXLkJhEvBks5ro_86gaJpZM4MaJ00> .

nemesifier self-requested a review March 14, 2017 09:22

nemesifier assigned gastonche Mar 14, 2017

nemesifier added the enhancement label Mar 14, 2017

nemesifier suggested changes Mar 16, 2017

View reviewed changes

nemesifier suggested changes Mar 22, 2017

View reviewed changes

[admin] added full screen mode to advanced json editor

8d3ce6e

nemesifier merged commit 41eb9ee into openwisp:master Mar 24, 2017

nemesifier mentioned this pull request Mar 24, 2017

[UI] Improve advanced editor widget #45

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[admin] added full screen mode to advanced json editor #43

[admin] added full screen mode to advanced json editor #43

gastonche commented Mar 11, 2017 •

edited by nemesifier

coveralls commented Mar 11, 2017 •

edited

gastonche commented Mar 12, 2017

nemesifier commented Mar 13, 2017

nemesifier commented Mar 13, 2017

gastonche commented Mar 13, 2017

nemesifier commented Mar 13, 2017

gastonche commented Mar 13, 2017

gastonche commented Mar 14, 2017 •

edited

nemesifier commented Mar 14, 2017

coveralls commented Mar 14, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

nemesifier left a comment

nemesifier Mar 16, 2017

nemesifier Mar 16, 2017

coveralls commented Mar 16, 2017 •

edited

nemesifier commented Mar 16, 2017

gastonche commented Mar 16, 2017 via email

nemesifier commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

nemesifier commented Mar 21, 2017

nemesifier left a comment •

edited

gastonche commented Mar 24, 2017 via email

coveralls commented Mar 24, 2017

gastonche commented Mar 24, 2017

gastonche commented Mar 24, 2017

nemesifier commented Mar 24, 2017 •

edited

nemesifier commented Mar 24, 2017

gastonche commented Mar 27, 2017 via email

[admin] added full screen mode to advanced json editor #43

[admin] added full screen mode to advanced json editor #43

Conversation

gastonche commented Mar 11, 2017 • edited by nemesifier

coveralls commented Mar 11, 2017 • edited

gastonche commented Mar 12, 2017

nemesifier commented Mar 13, 2017

nemesifier commented Mar 13, 2017

gastonche commented Mar 13, 2017

nemesifier commented Mar 13, 2017

gastonche commented Mar 13, 2017

gastonche commented Mar 14, 2017 • edited

nemesifier commented Mar 14, 2017

coveralls commented Mar 14, 2017 • edited

coveralls commented Mar 16, 2017 • edited

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier Mar 16, 2017

Choose a reason for hiding this comment

nemesifier Mar 16, 2017

Choose a reason for hiding this comment

coveralls commented Mar 16, 2017 • edited

nemesifier commented Mar 16, 2017

gastonche commented Mar 16, 2017 via email

nemesifier commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

nemesifier commented Mar 21, 2017

nemesifier left a comment • edited

Choose a reason for hiding this comment

gastonche commented Mar 24, 2017 via email

coveralls commented Mar 24, 2017

gastonche commented Mar 24, 2017

gastonche commented Mar 24, 2017

nemesifier commented Mar 24, 2017 • edited

nemesifier commented Mar 24, 2017

gastonche commented Mar 27, 2017 via email

gastonche commented Mar 11, 2017 •

edited by nemesifier

coveralls commented Mar 11, 2017 •

edited

gastonche commented Mar 14, 2017 •

edited

coveralls commented Mar 14, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

nemesifier left a comment •

edited

nemesifier commented Mar 24, 2017 •

edited