[CLOSED] #7276 Live Preview highlight customization

[core-ai-bot]

Issue by Worie
Sunday Nov 27, 2016 at 18:33 GMT
Originally opened as adobe/brackets#12949

---

Live Preview highlight customization (colors of highlight can be customized via user preferences)

I needed to easly customize the highlight color of Live Preview - and I've achieved it with the changes in the PR. I'm not sure if that's the desired way of doing so, as this is my first PR to Brackets.

1. RemoteFunctions takes whole config as a parameter instead just "experimental" flag.
2. There's a preference in which user can define desired styling of highlightning.
3. The view is updated when preferences change without breaking live dev session.
4. If no user preferences are defined, the default configuration is used.

It also implements #7276 -> Preview of Live Preview with margins and paddings showing

---

Worie included the following code: https://github.com/adobe/brackets/pull/12949/commits

[core-ai-bot]

Comment by Worie
Monday Nov 28, 2016 at 09:46 GMT

---

Thanks@petetnt ! I've applied the changes you mentioned. Unfortunetly it seems like I've broken the test, as Debug -> Run tests throws Uncaught TypeError: Cannot read property 'experimental' of undefined in SpecRunner.html:36 . I can't investigate on it now, though.

Also it looks like I've closed this PR 3 hours ago, is it automated process or did i just close it by accident?

[core-ai-bot]

Comment by petetnt
Monday Nov 28, 2016 at 09:48 GMT

---

These closing is not an automated process, I guess you closed it by accident but that's no biggie 👍

I can help you out with debugging the test failures in a while so we can land this 👍

[core-ai-bot]

Comment by Worie
Tuesday Nov 29, 2016 at 11:56 GMT

---

It isn't pushed yet, but I've created updateConfig function in RemoteFunctions.js so the changes in the brackets.json could be loaded to current LiveDev session.

Also I'd like to try to implement adobe/brackets#7276 in my free time, as I think it is an awesome case and it could play well with changes made in this PR.

Edit: pushed

[core-ai-bot]

Comment by Worie
Wednesday Nov 30, 2016 at 10:28 GMT

---

Preview of Live Preview with margins and paddings showing

So, I've done it :) If user did not specify his own preferences, it looks as designed in adobe/brackets#7276 . Also changing those values does not break current Live Development session.

You can also turn showing the margins/paddings by changing value of the flag showMarginPadding in preferences. Probably this could be hooked to some keyboard shortcut.

I could probably add one condition to check if there's any padding, so there wont be any borders showing when padding value is equal to 0 or something.

And the tests... Yeah.

[core-ai-bot]

Comment by Worie
Friday Dec 16, 2016 at 10:20 GMT

---

Realized that elements that have css transition on them act a bit buggy (margins and paddings do not animate, and it causes that you have to wait for the transition to finish, then re-select this specific node to see changes), perhaps I'll do something about it next week, we'll see.

But still, i have no idea how to get into fixing those tests - any help would be awesome, even pointing to some specific files or anything. :)

[core-ai-bot]

Comment by ficristo
Friday Dec 16, 2016 at 19:21 GMT

---

If you open Chrome at http://localhost:9234/, after clicked Debug \ Run Tests, there are two errors Uncaught TypeError: Cannot read property 'experimental' of undefined.
Their stack trace point to:
spec/HTMLInstrumentation-test.js:40
spec/RemoteFunctions-test.js:33

[core-ai-bot]

Comment by Worie
Thursday Dec 22, 2016 at 14:08 GMT

---

Due to getClientBoundingRect including the transform when calculating the size of element and its position (https://i.imgur.com/tNNjXEw.png), i get pretty weird and unintuitive result - http://imgur.com/MTwZcfB .

This is a pretty simple animation which rotates the element by 90degrees, as you can see the size of element is enlarged in preview due to usage window.getClientBoundingRect

I do not know how to bite this one without some heavy math calculations (nor I am any good at math).

Perhaps some of you have an idea how to get the current implementation to act like a chrome one easly?

[core-ai-bot]

Comment by petetnt
Friday Dec 23, 2016 at 08:09 GMT

---

@Worie I was thinking about that but didn't find any simple solutions yet, could you share an sample project / file that shows the behaviour in the GIF?

[core-ai-bot]

Comment by Worie
Friday Dec 23, 2016 at 18:57 GMT

---

Perhaps this will be enough: https://gist.github.com/Worie/caae957b805f08cb733f51406d32aabe

Also, now every transform function is applied to the highlight div - probably only scale and rotate should be considered.

[core-ai-bot]

Comment by Worie
Saturday Jan 28, 2017 at 19:18 GMT

---

I've started a little refactor of this code. I'll not focus on the transform issue atm, though.

[core-ai-bot]

Comment by Worie
Saturday Jan 28, 2017 at 21:16 GMT

---

@petetnt In latest commit, I've removed the sum function we introduced during this PR. I felt that it was making more harm than good and made it less clear what exacly is happening there.

Of course, I can revert this change and apply the sum function to current implementation. But I guess it's not really necessary.

[core-ai-bot]

Comment by petetnt
Wednesday Feb 01, 2017 at 13:24 GMT

---

Hey Worie,

sorry for the lack of comms, been a bit busy as of late. Is this PR ready for another round of review?

[core-ai-bot]

Comment by Worie
Wednesday Feb 01, 2017 at 13:40 GMT

---

Yeah I guess. If we could only somehow invoke the inspect on element as we can in Chrome Dev tools from RemoteFunctions this PR would be much shorter :/ But I don't think it is possible atm.

Anyway - the workaround for transform issue is that we simply do not apply transform to our custom highlightning divs. That means that they dont rotate nor scale, but translation works pretty good on them.

I guess that we could perhaps split this into two issues - close this one when the code quality is good enough, and then take care of the transform issue.

Because, I think that it might take a while to do (and I have no idea atm how, but I guess we'd need to get the data about the transformations from matrix3d and reverse it to something like rotate(45deg) translateX()...)

[core-ai-bot]

Comment by petetnt
Wednesday Feb 15, 2017 at 14:53 GMT

---

I think the scope is just fine right now, I could run another code review and test it out again myself and then merge it in (and create a issue for tracking the transform part).@ficristo you could also take a look if you want to 👍

[core-ai-bot]

Comment by ficristo
Wednesday Feb 15, 2017 at 14:57 GMT

---

I think@saurabh95 is more indicated