Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Fix #737 #857

Merged
merged 2 commits into from
Nov 7, 2020
Merged

Fix #737 #857

merged 2 commits into from
Nov 7, 2020

Conversation

N1KN1M
Copy link
Contributor

@N1KN1M N1KN1M commented Nov 7, 2020

Closes #737

Previously, all icons were in a single row, which became bothersome when scaling.

@razaamir
Copy link

razaamir commented Nov 7, 2020 via email

@N1KN1M N1KN1M marked this pull request as draft November 7, 2020 11:46
@N1KN1M N1KN1M marked this pull request as ready for review November 7, 2020 11:47
@N1KN1M
Copy link
Contributor Author

N1KN1M commented Nov 7, 2020

@GitSquared I tried adding you as a reviewer / assigning it to you but it seems like PR authors don't have such permissions in github.

@razaamir
Copy link

razaamir commented Nov 7, 2020 via email

@GitSquared
Copy link
Owner

@GitSquared I tried adding you as a reviewer / assigning it to you but it seems like PR authors don't have such permissions in github.

No need, i'm notified either way. Will review this ASAP, thanks!

@GitSquared
Copy link
Owner

GitSquared commented Nov 7, 2020

(nvm, see review below)

@razaamir
Copy link

razaamir commented Nov 7, 2020 via email

@@ -149,7 +149,7 @@ section#filesystem.list-view > div#fs_disp_container:not(.disks) > div > h4:nth-
section#filesystem.list-view > div#fs_disp_container:not(.disks) > div > h4:nth-of-type(3) { width: 38%; }

div#fs_disp_container.disks {
display: flex;
padding: 0 0.5vw;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check line :109 of the same file, when list view (Ctrl+Shift+L) is enabled this will break.

Perhaps using flex-wrap instead of deactivating flexbox would be easier, and might also retain the center alignment of the disk view items? 🤔

Copy link
Contributor Author

@N1KN1M N1KN1M Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry about that; wasn't aware of list view 😅
Actually, simply adding in flex-wrap brings the list view for disks to be pretty much the same as the normal grid view (the only slight difference being spacing between border and items due to different grid-gap attributes).
Here's what both views look like on my screen(with flex-wrap):

Grid View:
grid_view

List View:
list_view

Is that alright or is it preferred to have horizontal scrolling on the original disk view instead?

Copy link
Owner

@GitSquared GitSquared Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's good! Can you scroll down if there are multiple screens of disks though? Just want to make sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'm able to scroll after copy pasting a couple of divs in dev tools.

…s with disks. Simply using the inherited grid display breaks for list views.
@GitSquared
Copy link
Owner

All good, thanks for taking the time to work on this!

@GitSquared GitSquared changed the title Changed file system disk layout to grid Fix #857 Nov 7, 2020
@GitSquared GitSquared merged commit 56de649 into GitSquared:master Nov 7, 2020
@GitSquared GitSquared changed the title Fix #857 Fix #737 Nov 7, 2020
@N1KN1M
Copy link
Contributor Author

N1KN1M commented Nov 7, 2020

Thanks Gaby, glad I can be of help!

@razaamir
Copy link

razaamir commented Nov 8, 2020 via email

@lebarde lebarde mentioned this pull request Nov 25, 2020
18 tasks
eugene2candy pushed a commit to eugene2candy/edex-ui that referenced this pull request Apr 10, 2021
* Changed disk layout to use inherited grid layout, added some padding for spacing from border.

* Added back display: flex and added flex-wrap to account for list views with disks. Simply using the inherited grid display breaks for list views.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug report] Messy hard disk UI
3 participants