Add ability to have actions pulldown menu in pfDataList directive #95
Conversation
jeff-phillips-18
force-pushed
the
master
branch
from
f7185b0 to
37d6d70
Compare
There was a problem hiding this comment.
Constructing HTML using javascript is difficult to read and maintain and is somewhat of an AngularJS anti-pattern. We should be using angular-bootstrap's dropdown directive, wrapping it with modifications if desired.
There was a problem hiding this comment.
The problem with that is that it would create the dropdown for every item in the list. I did this so that the dropdown menu was only created when the item was selected. If there are 1000's of rows the performance would suffer creating all these menus.
There was a problem hiding this comment.
The problem with that is that it would create the dropdown for every item in the list.
What about using ng-if and the aforementioned directive?
There was a problem hiding this comment.
That's a good idea I hadn't thought of. I will do that and see how it goes.
There was a problem hiding this comment.
That worked well and simplified the unit tests greatly. Thanks!
|
Should we use absolutely position the dropdown? See the dropdown barely visible at the bottom of the attached screenshot.
|
jeff-phillips-18
force-pushed
the
master
branch
from
f02a51d to
9a264e4
Compare
dist/angular-patternfly.js
Outdated
There was a problem hiding this comment.
Think we have a style for this now
duh, right 😊
|
LGTM |
@jeff-phillips-18 thoughts on this? |
|
@waldenraines Yeah, I agree and I'm working on it. |
dist/styles/angular-patternfly.css
Outdated
There was a problem hiding this comment.
perfect chance to save some bytes #222 :)
|
@waldenraines I've got it working. The only issue I'm seeing is that the dropdown menu will stay put if you scroll the window while it is up. That is, if I click the link button to see the menu, then I use the wheelie mouse to scroll the page, the dropdown menu does not move with the item it was displayed for. Any thoughts? Acceptable? |
|
This solution is going to have problems if the menu is rendered near the bottom of the page. |
|
lol nm looks like @waldenraines is already on it. |
|
We need to have logic that figures out if it is close to the bottom of the page and if so have it show up and not down. |
This isn't yet committed is it? I was going to test this out to see it in action but I still see the same behavior. If I understand correctly, I wonder if we should just close any open dropdowns on scroll? |
|
I have not yet pushed. I will so you can see what I mean. |
dist/styles/angular-patternfly.css
Outdated
There was a problem hiding this comment.
instead of ul use .dropdown-menu it is more performant
@jeff-phillips-18 yeah I think closing the menu on scroll would be the best bet here. I am open to other suggestions though. |
|
So what happens if the data list is wider than the space that is available. Does the page scroll, datalist wrapper? Do we just lose the actions menu? |
|
Yeah, I think closing the menu on scroll is the best option as well. Just added a commit for that. If the list scrolls or the window scrolls, the menu is hidden. I will squash commits when ready before merge. |
@jeff-phillips-18 what about @erundle's concern? |
|
@erundle @waldenraines Data list is designed to not scroll horizontally. That is the pattern's design. |
jeff-phillips-18
force-pushed
the
master
branch
from
7a3d97f to
2700545
Compare
|
Squashed. |
src/views/datalist/data-list.html
Outdated
There was a problem hiding this comment.
FYI I am looking at this directive in self-service and we are using class="row" too much. Everytime that is used we get margin: 0 -20px. This is causing scrollbars. Please only use the class of row if it is directly inside a container or container-fluid div
There was a problem hiding this comment.
In this particular case, row is needed as we use the margin areas for the checkbox and actions menu.
There was a problem hiding this comment.
Ok well this margin pulls things closer to the edge of the page. The way you have things currently structured is you are making this element wider than the containing parent div. Maybe your PR will address this but if not we need to look at restructuring somethings. Just putting overflow-x:hidden isn't the solution. We need to stop it from growing wider than its container to begin with.
There was a problem hiding this comment.
If you add this: .data-list-pf .list-group-item { padding: 10px 0 }; and remove the row class from that line things seem to render better.
There was a problem hiding this comment.
Are you doing something different in your parent container? Making this change causes the list example in ngdoc unreadable.
There was a problem hiding this comment.
OK, I found the issue and removed the row class. Thanks @erundle !
jeff-phillips-18
force-pushed
the
master
branch
from
2700545 to
f8d643e
Compare
|
@erundle Are you ok with that change? |
|
Yes will merge |
Add ability to have actions pulldown menu in pfDataList directive
4 participants
This adds a dropdown menu button to the far right of the items in the data list if actions are provided to the pfDataList directive.