Toolbar #36

RvanderLaan · 2019-04-04T21:33:52Z

Closes #29
I tried to use the grid but it's a little more difficult than I hoped to make it act like the UI prototype.
It mostly contains placeholder elements, except for toggling the inspector+outliner (now with transitions!), the settings button and the sorting button (which only opens a menu).

Grids are hard. Might need some refactoring.

schroef · 2019-04-05T01:45:25Z

The way it's done in the prototype is simply by toggling classes. I've setup 4 classes which will change the look and feel of the results.

So when I click grid, first it deletes all 4 class, just to make sure none are than, than only the active one from the button is added. All elements inside a result div react to this class

…t form

hummingly · 2019-04-06T11:53:45Z

I had a look yesterday. To solve the problem we should see the toolbar as a single entity. As I understand it, the toolbar is always going to be above the content and it will only move as the windows is resized.
This means we create a container div around the toolbar components that has either display: grid or display: flex. We also wrap <main>, <nav> and <aside> in a div and move the layoutContainer id to it. I haven't checked yet how you make the settings panel. Is it an overlay or should it be inside

?

RvanderLaan · 2019-04-06T12:19:13Z

Yep, that sounds good, I'll try it out. I was struggling a little bit with the CSS grid as I've never used it before so at the moment the toolbar buttons go all over the place.
The settings panel is a Drawer component from Blueprint, basically just an overlay on the right side. They added it only recently so I had to update the dependencies

…een when an error occurs

hummingly · 2019-04-06T21:35:15Z

I totally forgot that content can be spread over several cells. The toolbars is the whole first row and uses flex-box now. I noticed one issue is that there is never a hotizontal scrollbar in main. The problem is that the height must be set to an absolute value but the toolbar takes space too.

schroef · 2019-04-06T21:53:03Z

When you use ID you can pin-point exacty where each div goes in the toolbar. Thats how we did it in the prototype aswell.

As for the settings panel, this is indeed a div which is set to display:none. That is the same as i did the search panel and the depricated locations panel.

schroef · 2019-04-06T21:58:39Z

PS indeed i think that drawer function looks the same. Cant jquery be used if this wont work?
How are you tackling the panels for tag and search?

schroef · 2019-04-06T22:06:33Z

I totally forgot that content can be spread over several cells. The toolbars is the whole first row and uses flex-box now. I noticed one issue is that there is never a hotizontal scrollbar in main. The problem is that the height must be set to an absolute value but the toolbar takes space too.

To hide a scrollbar you either use this for the content are
overflow: scroll;

If a scrollbar shows in other areas you can force hide it with this;
Here's more info about overflow, https://developer.mozilla.org/en-US/docs/Web/CSS/overflow

hummingly · 2019-04-06T23:39:48Z

Yeah I did this already but the height of the scrollbar depends on the height of the element. <main> inherits through its parents the height of <html> which is equivalent to 100vh. The scrollbar is properly shown when the toolbar is removed.

Notice the missing scrollbar element at the bottom.

Without the scrollbar it works.

The easiest way to resolve this and make the horizontal scrollbar visible is to calculate the height by setting the height of the toolbar and subtracting this value from the height. However, this should be done when the icons are finished.

schroef · 2019-04-07T04:16:11Z

@hummingly
Why would we want to have a horizontal scrollbar. The content area will adjust to the visible area right? Doesnt everybody hate that because it feels unnatural?

Im not sure i see any difference in that scrollbar track and thumb in these 2 images. In both images i see a scroll track and in both i believe i see an scroll-thumb. Isnt that the small rectangle at the top? But i guess you meant the missing horizontal one.

PS i added a custom scrollbar to my mockup and it works fine. You need to set the overflow-y:scroll to the content area.

Why does it need height actually? In my prototypes ive set the overflow area to initial. In order to get this to work the content area needs an extra div which i call forced-overflow. This has has these settings;

   overflow: initial;

You can check my prototype v10 and also this screengrab

I can also added this custom scrollbar to a select field field the tagging and also in the settings

schroef · 2019-04-07T04:24:36Z

After comparing the images fullscreen i thing i understand what you meant. I also think by accident you typed horizontal scrollbar while meaning vertical. You mean the track height isnt correct in the first image. I still think that due overflow not set, in the second image you did add that and than it show fine. though you did need to delete the toolbar, im not sure why though.

hummingly · 2019-04-07T12:39:27Z

I opened the html file by default with Firefox which shows the default scrollbar and the arrow at the bottom is also missing. But openening in Chrome shows your custom scroll bar which is why it does not matter.
Horizontal scrolling is only for the extreme resizing case where content area's width is smaller than an image. In your mockup, you use absolute position and also set the height of the toolbar.

Now I have set a height for the toolbar and subtract it from the height. This fixes the issue.

src/renderer/frontend/components/Outliner.tsx

schroef · 2019-04-08T16:36:15Z

Why does it need a height value to show the scrollbar? Mine works without setting any height. Because if it does need a height, than on resize you need to recalculate it otherwise it will break the scrollbar.

hummingly · 2019-04-08T21:12:59Z

Both images have overflow set. As you noted in the second image the scrollbar is correctly sized. The reason is that the height of the content area is 100vh (inherited from body), which means without toolbar it shows it correctly. But with the toolbar the total height is 100vh + height of toolbar which is greater than the viewport height (100vh). The scrollbar is moved down by the toolbar. That's why that arrow is missing.
It is only an issue when the viewport width is too small. Then there would be no horizontal scrollbar visible because it is below the viewport which would confuse the user.
For the toolbar I thought about preventing the icons from wrapping and let it overflow.

hummingly · 2019-04-09T19:00:19Z

Setting the height in the grid is better. I haven't done anything about the behaviour of the toolbar on resize yet but that can be done later.

hummingly · 2019-04-09T18:43:56Z

src/renderer/frontend/stores/FileStore.ts

@@ -73,6 +74,15 @@ class FileStore {
    }
  }

+  @action
+  async loadLocation(dir: string) {
+    await this.clearFileList();


Why is there an await? clearFileList is unlike removeFile not marked as async.

hummingly · 2019-04-09T18:46:28Z

src/renderer/frontend/stores/FileStore.ts

@@ -73,6 +74,15 @@ class FileStore {
    }
  }

+  @action
+  async loadLocation(dir: string) {


Exception handling is missing which causes on my pc an exception since I moved those folders to another drive.

hummingly · 2019-04-09T18:52:59Z

src/renderer/frontend/components/LocationsTree.tsx

+if (platform === 'win32') { // Windows
+} else if (platform === 'darwin') { // Mac
+} else if (platform === 'linux') { // Linix
+} else {


Electron only supports windows, linux and macos and there are also no plans for us to ship to other platforms right now. A switch statement would be better.

hummingly · 2019-04-09T18:53:12Z

src/renderer/frontend/components/LocationsTree.tsx

+// https://www.npmjs.com/package/platform-folders
+if (platform === 'win32') { // Windows
+} else if (platform === 'darwin') { // Mac
+} else if (platform === 'linux') { // Linix


hummingly · 2019-04-09T19:18:42Z

src/renderer/frontend/components/LocationsTree.tsx

+const platform = os.platform();
+const homeDir = os.homedir();
+
+const systemDirs = {


Could be an enum

hummingly · 2019-04-09T19:19:03Z

src/renderer/frontend/stores/UiStore.ts

@@ -29,8 +29,9 @@ class UiStore {
  @observable theme: 'LIGHT' | 'DARK' = 'DARK';

  // UI
-  @observable isOutlinerOpen: boolean = true;
+  @observable outlinerPage: 'LOCATIONS' | 'TAGS' | 'SEARCH' = 'TAGS';


Could be an enum

RvanderLaan · 2019-04-10T21:52:39Z

I've replaced the Locations tab with an Import tab for now as we agreed on in the meeting, so most of that code has been removed (but I agree with your comments @hummingly !)
About the enums/strings, I've noticed that many people prefer using string literals over enums (like Blueprint). Is there a specific reason you prefer enums?

hummingly · 2019-04-10T22:40:48Z

I'm just used to using a lot of enums and it was more meant as a suggestion. It is mostly to prevent small typos as mistyping the name of an enum variant will result in an error. If we're going to use those string literals in a lot of places, it would be better to use the auto-completion than copy and pasting.

RvanderLaan · 2019-04-11T09:03:50Z

The cool thing about Typescript is that string literals have that behavior too, without having to import anything :D

…rent tag selection

RvanderLaan · 2019-04-11T20:58:52Z

Is it alright to merge this PR? It would be nice to make a release before the meeting. We'd also have to merge #32 which has some conflicts with this one

hummingly · 2019-04-11T21:03:47Z

Let me take a look. I would merge the toolbar branch and #23 into master first and then rebase #32.

src/renderer/frontend/App.tsx

hummingly · 2019-04-11T22:39:33Z

Closes #29

Added Toolbar component with mostly placeholder elements

b053475

Grids are hard. Might need some refactoring.

RvanderLaan changed the title ~~Added Toolbar component with mostly placeholder elements~~ Toolbar Apr 4, 2019

Remi van der Laan added 2 commits April 5, 2019 20:55

Switch between outliner pages

bc31b49

Added (de)select all and layout popover menu

2234bc4

hummingly added this to the Alpha v. 1.0 milestone Apr 5, 2019

Added basic Locations component, added non-functional Search componen…

8406f68

…t form

Remi van der Laan and others added 3 commits April 6, 2019 15:47

Added ErrorBoundary that shows an error screen instead of a blank scr…

3b695a7

…een when an error occurs

Improved toolbar css

df653f7

Use IDs and grid areas for the toolbar

3ef9a5f

Fixes scrollbar height

dc008b4

hummingly reviewed Apr 7, 2019

View reviewed changes

src/renderer/frontend/components/Outliner.tsx Show resolved Hide resolved

Cleanup height

62ec0fd

hummingly requested changes Apr 9, 2019

View reviewed changes

Replaced Locations with Import tab, small fixes

04753c2

Remi van der Laan added 3 commits April 11, 2019 22:06

Crash fix: Deselect files that are not tagged with any tag in the cur…

f419bd2

…rent tag selection

Added query overview at the top of the gallery

dec16db

Removed selection header, replaced with toolbar actions

c81170d

Always re-fetch files when tag selection is updated

d32f54c

hummingly reviewed Apr 11, 2019

View reviewed changes

src/renderer/frontend/App.tsx Outdated Show resolved Hide resolved

hummingly approved these changes Apr 11, 2019

View reviewed changes

Formatting

db29500

hummingly merged commit f737aa3 into file-select-and-opening Apr 11, 2019

hummingly deleted the toolbar branch April 11, 2019 22:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Toolbar #36

Toolbar #36

RvanderLaan commented Apr 4, 2019 •

edited

Loading

schroef commented Apr 5, 2019

hummingly commented Apr 6, 2019

RvanderLaan commented Apr 6, 2019

hummingly commented Apr 6, 2019

schroef commented Apr 6, 2019

schroef commented Apr 6, 2019

schroef commented Apr 6, 2019

hummingly commented Apr 6, 2019

schroef commented Apr 7, 2019 •

edited

Loading

schroef commented Apr 7, 2019

hummingly commented Apr 7, 2019

schroef commented Apr 8, 2019

hummingly commented Apr 8, 2019

hummingly commented Apr 9, 2019

hummingly Apr 9, 2019

hummingly Apr 9, 2019

hummingly Apr 9, 2019

hummingly Apr 9, 2019

hummingly Apr 9, 2019

hummingly Apr 9, 2019

RvanderLaan commented Apr 10, 2019

hummingly commented Apr 10, 2019

RvanderLaan commented Apr 11, 2019

RvanderLaan commented Apr 11, 2019

hummingly commented Apr 11, 2019

hummingly commented Apr 11, 2019 •

edited

Loading

Toolbar #36

Toolbar #36

Conversation

RvanderLaan commented Apr 4, 2019 • edited Loading

schroef commented Apr 5, 2019

hummingly commented Apr 6, 2019

RvanderLaan commented Apr 6, 2019

hummingly commented Apr 6, 2019

schroef commented Apr 6, 2019

schroef commented Apr 6, 2019

schroef commented Apr 6, 2019

hummingly commented Apr 6, 2019

schroef commented Apr 7, 2019 • edited Loading

schroef commented Apr 7, 2019

hummingly commented Apr 7, 2019

schroef commented Apr 8, 2019

hummingly commented Apr 8, 2019

hummingly commented Apr 9, 2019

hummingly Apr 9, 2019

Choose a reason for hiding this comment

hummingly Apr 9, 2019

Choose a reason for hiding this comment

hummingly Apr 9, 2019

Choose a reason for hiding this comment

hummingly Apr 9, 2019

Choose a reason for hiding this comment

hummingly Apr 9, 2019

Choose a reason for hiding this comment

hummingly Apr 9, 2019

Choose a reason for hiding this comment

RvanderLaan commented Apr 10, 2019

hummingly commented Apr 10, 2019

RvanderLaan commented Apr 11, 2019

RvanderLaan commented Apr 11, 2019

hummingly commented Apr 11, 2019

hummingly commented Apr 11, 2019 • edited Loading

RvanderLaan commented Apr 4, 2019 •

edited

Loading

schroef commented Apr 7, 2019 •

edited

Loading

hummingly commented Apr 11, 2019 •

edited

Loading