-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IBX-895: As an Editor, I want to see SubItems redesigned #1923
Conversation
@@ -45,30 +24,13 @@ | |||
left: 0; | |||
width: calculateRem(30px); | |||
line-height: 0; | |||
margin-top: -0.5px; // Fixes borders alignment on Safari and Edge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: it seems that we no longer need this. Checked on latest Safari and Edge on HD and Retina displays.
89aa53f
to
d1bc7d6
Compare
src/bundle/ui-dev/src/modules/sub-items/components/table-view/table.view.item.component.js
Outdated
Show resolved
Hide resolved
f369881
to
3801866
Compare
@@ -1,5 +1,5 @@ | |||
(function(global, doc, $, eZ) { | |||
const tables = doc.querySelectorAll('.ibexa-table'); | |||
const tablesWithAutomaticCheckbox = doc.querySelectorAll('.ibexa-table:not(.ibexa-table--no-automatic-main-checkbox)'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change automatic checkbox to bulk checkbox (as discussed in private)
@@ -1,5 +1,5 @@ | |||
(function(global, doc, $, eZ) { | |||
const tables = doc.querySelectorAll('.ibexa-table'); | |||
const tablesWithAutomaticCheckbox = doc.querySelectorAll('.ibexa-table:not(.ibexa-table--no-automatic-main-checkbox)'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also instead of .ibexa-table--no-bulk-checkbox
use .ibexa-table--has-bulk-checkbox
to align with cell--has-checkbox (also discuessed in private)
display: flex; | ||
flex-direction: column; | ||
padding: calculateRem(16px); | ||
height: calculateRem(238px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure we want to have fixed height? What if title or sth will be multiline?
|
||
attrs.disabled = true; | ||
} | ||
const label = Translator.trans(/*@Desc("Upload")*/ 'multi_file_upload_open_btn.label.2', {}, 'multi_file_upload'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 2
in id? That's very odd
@@ -30,7 +30,7 @@ const SORTKEY_MAP = { | |||
priority: 'LocationPriority', | |||
}; | |||
const TABLE_CELL_CLASS = 'c-table-view__cell'; | |||
const TABLE_HEAD_CLASS = `${TABLE_CELL_CLASS} ${TABLE_CELL_CLASS}--head`; | |||
const TABLE_HEAD_CLASS = `ibexa-table__header-cell ${TABLE_CELL_CLASS} ${TABLE_CELL_CLASS}--head`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would remove TABLE_CELL_CLASS
const, it makes these four places where it's used more unreadable, but it's only suggestion
6809cd9
to
506e8c7
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
New look
Checklist:
$ composer fix-cs
)