Merge new box-model code into QtWebEngine conversion #3618

ferdnyc · 2020-07-21T00:05:44Z

See #3617 for PR details. This is the same source branch, but targeting the WebEngine PR branch instead of develop.

- Remove border styling from clip, transition objects - Remove box-sizing: border-box, which made objects' DOM size unreliable - Create new CSS class .tl-border, to hold border styling - Insert new div.tl-border container just inside each clip/transition - Remove sizing-bug "fudge factors" (like bounding_box_padding) - Implement all highlight/hover styling in pure CSS (with `:hover`), eliminating `mouseover` event handlers - Apply selected-item border styling to transitions, as well as clips

(I hate when it has a point.)

ferdnyc · 2020-07-21T20:19:09Z

src/timeline/index.html

+				<div ng-hide tl-clip ng-repeat="clip in project.clips" id="clip_{{clip.id}}" ng-click="selectClip(clip.id, true, $event)" ng-right-click="showClipMenu(clip.id, $event)" class="clip timeline_obj droppable" ng-class="getClipStyle(clip)" style="width:{{(clip.end - clip.start) * pixelsPerSecond}}px; left:{{clip.position * pixelsPerSecond}}px; top:{{getTrackTop(clip.layer)}}px;z-index:{{1000 + $index}};">
+				<div id="clip_border_{{clip.id}}" class="clip_border tl-border">
 					<div class="clip_top">


As small a thing as it is, inserting the extra <div class="tl-border"> is the big thing that makes all the difference.

By moving the borders to that <div> and unsetting box-sizing:border-box on the outer <div tl-clip>, the borders no longer alter the dimensions/placement of the main <div tl-clip>, which means all of the sizing/placement math becomes predictable and repeatable.

ferdnyc · 2020-07-21T20:20:25Z

src/timeline/index.html

 					<div class="clip_top">
-						<div tl-clip-menu class="clip_menu" ng-show="!enable_razor" ng-mousedown="showClipMenu(clip.id, $event)" tooltip-enable="!enable_razor" tooltip="{{clip.title}}" tooltip-placement="bottom" tooltip-popup-delay="400"></div>
+						<div tl-clip-menu class="clip_menu menu_button" ng-show="!enable_razor" ng-mousedown="showClipMenu(clip.id, $event)" tooltip-enable="!enable_razor" tooltip="{{clip.title}}" tooltip-placement="bottom" tooltip-popup-delay="400"></div>


All of the menus were using the same styling, so I created a shared .menu_button { } CSS stanza and applied it to all of them, eliminating the duplicated CSS.

ferdnyc · 2020-07-21T20:24:27Z

src/timeline/js/directives/clip.js

-
-      //handle hover over on the clip
-      element.hover(
-        function () {
-          if (!dragging) {
-            element.addClass("highlight_clip", 200, "easeInOutCubic");
-          }
-        },
-        function () {
-          if (!dragging) {
-            element.removeClass("highlight_clip", 200, "easeInOutCubic");
-          }
-        }
-      );
-


I knew I really wanted to get rid of this, because it was fighting with the rest of the CSS rules. You could even see it happening in the UI, clips would get into a state where mousing between them would cause borders to flash, or disappear momentarily and reappear — all sorts of issues. Plus, :hover styling is at this point a basic CSS feature, there's no reason it couldn't be used to create the hover effects without needing to add any extra JavaScript code.

ferdnyc · 2020-07-21T20:25:48Z

src/timeline/js/functions.js

+    var [nearby_offset, snapline_position]
+      = scope.getNearbyPosition(
+        [bounding_box.left, bounding_box.right],
+        10.0, bounding_box.selected_ids);
+  // var nearby_offset = results[0];
+  // var snapline_position = results[1];


Heh, forgot I even did that. Our JS code creates way too many temporary variables, as it's tossing around data. This eliminates one temporary array, which is something.

ferdnyc · 2020-07-21T20:27:31Z

src/timeline/js/functions.js

-  var bounding_box_padding = 3; // not sure why this is needed, but it helps line everything up
-  var results = scope.getNearbyPosition([bounding_box.left, bounding_box.right + bounding_box_padding], 10.0, bounding_box.selected_ids);


Doing away with box-sizing: border-box let me eliminate the bounding_box_padding adjustment to the lasso processing, which was only there (whether that comment knew it or not) to correct for the borders being included as part of the object dimensions.

ferdnyc · 2020-07-21T20:30:39Z

src/timeline/index.html

-				<div ng-repeat="layer in project.layers.slice().reverse()" id="track_{{layer.number}}" ng-right-click="showTimelineMenu($event, layer.number)"  class="{{getTrackStyle(layer.lock)}}" style="width:{{getTimelineWidth(1024)}}px;">
+				<div ng-repeat="layer in project.layers.slice().reverse()" id="track_{{layer.number}}" ng-right-click="showTimelineMenu($event, layer.number)" class="{{getTrackStyle(layer.lock)}}">


Another unexpected optimization: I noticed we were forcing the width of every track DIV with inline styling. But they're all in a container that's also forced with the exact same width. So I just added width: 100% to the track CSS, removed the inline styling, and let the container dictate the width of the tracks.

ferdnyc · 2020-07-21T20:35:39Z

src/timeline/media/css/main.css

+  border-top-right-radius: 8px;
+  border-bottom-right-radius: 8px;
+  box-shadow: 0 0 10px #000;
+  box-sizing: border-box;


Ironically, the problem with vertical placement of the clips/transitions was the lack of border-box sizing on the track DIVs, which weren't the same size we expected them to be due to their borders pushing them out of place. We don't actually care about the size of the tracks themselves, we just care that they're in the positions we expect them to be. So, using box-sizing: border-box ensures that the (bordered) track containers will be the same size as the (now-unbordered) draggables, and they line up correctly.

ferdnyc · 2020-07-21T20:40:43Z

src/timeline/media/css/main.css

+/* Selected item border, semi-transparent unless hovered */
+.ui-selected>.tl-border {
+  border-color: #ff0000a8;
+}
+
+.ui-selected:hover>.tl-border {
+  border-color: #ff0000ff;
+}


Because the <div tl-track> is a container for <div class="tl-border">, which is in turn a container for all of the rest of its contents, my initial attempts at dicking around with setting opacity to create hover effects all failed. I couldn't change the opacity of the border <div> without everything inside it also becoming semi-transparent. So, I finally hit on the idea of just assigning semi-transparent colors to the borders, in the non-hovered state.

ngClass supports an object as argument, each key for which the value evaluates to true will be used as a class name.

…bengine-box-model

- New 32x32 cursors (which fit new Chrome/Firefox restrictions) will show the dashed line only when hovering over clips/transitions, just the "knife" part otherwise - New reveal logic relies on setting a class on the body element, seems to style things appropriately. Some UI elements (resize handles, the playhead) will show other cursors, if still active in razor mode. - The (re-laid-out) debug panel now includes a razor mode toggle.

ferdnyc added 5 commits July 20, 2020 12:45

timeline: Fix id for ruler markers container

cfbeb0e

Timeline: box-sizing, width:100% for track DIVs

39aff20

Merge branch 'develop' into box-model

c5683f6

Timeline: Shared CSS class styles all menu icons

21cde98

ferdnyc force-pushed the box-model branch from 81095e3 to 24aaa0f Compare July 21, 2020 00:44

CSS fixes to appease Codacy

bda8e86

(I hate when it has a point.)

ferdnyc force-pushed the box-model branch from 24aaa0f to bda8e86 Compare July 21, 2020 01:12

timeline: Don't interpolate getThumbPath in ng-show

6328779

ferdnyc commented Jul 21, 2020

View reviewed changes

ferdnyc added 5 commits July 22, 2020 00:48

Move tlMultiSelect directive to separate file

bc24a64

Move ng-cloak styling to HTML head

438f22a

Eliminate getFooStyle() JS functions in ng-class

0b155e8

ngClass supports an object as argument, each key for which the value evaluates to true will be used as a class name.

More CSS fixes

013791b

timeline: Remove transition hover effect JS

18c0210

ferdnyc force-pushed the box-model branch from 6f27287 to 18c0210 Compare July 23, 2020 12:26

ferdnyc added 3 commits July 23, 2020 08:34

Merge branch 'new-webengine-support' into webengine-box-model

c5d56e1

timeline CSS: Format, fix ordering of rules

5b92dd1

multiselect.js: Comment-declare globals

ae87a50

ferdnyc mentioned this pull request Jul 23, 2020

Initial support for QtWebEngine and Async callbacks for timeline/Qt Integration #3604

Merged

ferdnyc added 5 commits July 28, 2020 06:01

Make SVG first default for razor, update styling

3a71845

Merge remote-tracking branch 'upstream/new-webengine-support' into we…

5d8a1f6

…bengine-box-model

Fix order of hover styling in CSS

17ffd01

Remove razor-tool icon from resource file

3423bf7

ferdnyc force-pushed the webengine-box-model branch from 643debb to 3423bf7 Compare July 31, 2020 00:26

jonoomph closed this Aug 17, 2020

ferdnyc mentioned this pull request Nov 22, 2020

Timeline CSS: More visible keyframe marks #3842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge new box-model code into QtWebEngine conversion #3618

Merge new box-model code into QtWebEngine conversion #3618

ferdnyc commented Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

ferdnyc Jul 21, 2020

		var bounding_box_padding = 3; // not sure why this is needed, but it helps line everything up
		var results = scope.getNearbyPosition([bounding_box.left, bounding_box.right + bounding_box_padding], 10.0, bounding_box.selected_ids);

		<div ng-repeat="layer in project.layers.slice().reverse()" id="track_{{layer.number}}" ng-right-click="showTimelineMenu($event, layer.number)" class="{{getTrackStyle(layer.lock)}}" style="width:{{getTimelineWidth(1024)}}px;">
		<div ng-repeat="layer in project.layers.slice().reverse()" id="track_{{layer.number}}" ng-right-click="showTimelineMenu($event, layer.number)" class="{{getTrackStyle(layer.lock)}}">

Merge new box-model code into QtWebEngine conversion #3618

Merge new box-model code into QtWebEngine conversion #3618

Conversation

ferdnyc commented Jul 21, 2020

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