Skip to content
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

Remove the gridfield item count from the top of gridfields when there is no header #256

Open
1 of 2 tasks
clarkepaul opened this issue Sep 14, 2017 · 10 comments
Open
1 of 2 tasks

Comments

@clarkepaul
Copy link
Contributor

clarkepaul commented Sep 14, 2017

The gridfield item count is not required when there is no gridfield title directly above the grid as it causes an unwanted space. By removing the view count at the top the gridfield actions will be closer to the gridfield providing a clearer UX.

We could also remove it all together (whether there is a header or not), but it doesn't seem to do any harm when there is a header present.
pasted_image_15_09_17__10_38_am

Pull request

@tractorcow
Copy link
Contributor

The gridfield item count is not required when there is no gridfield title directly above the grid as it causes an unwanted space.

What do you mean "not required"? Why is it required when there is a title?

@tractorcow
Copy link
Contributor

I would rather re-arranging the header so that it could fit without it being removed. It's important information that affects information communicated to the content editor; They now need to scroll to the bottom to see how many items are in this list.

@tractorcow
Copy link
Contributor

Alternate solution:

Before:

image.png

After:

image.png

diff --git a/src/Forms/GridField/GridFieldConfig_RecordEditor.php b/src/Forms/GridField/GridFieldConfig_RecordEditor.php
index 8e30b3d02..ea24e5238 100644
--- a/src/Forms/GridField/GridFieldConfig_RecordEditor.php
+++ b/src/Forms/GridField/GridFieldConfig_RecordEditor.php
@@ -29,7 +29,7 @@ class GridFieldConfig_RecordEditor extends GridFieldConfig
         }
         $this->addComponent(new GridFieldEditButton());
         $this->addComponent(new GridFieldDeleteAction());
-        $this->addComponent(new GridFieldPageCount('toolbar-header-right'));
+        $this->addComponent(new GridFieldPageCount('buttons-before-right'));
         $this->addComponent($pagination = new GridFieldPaginator($itemsPerPage));
         $this->addComponent(new GridFieldDetailForm());

@clarkepaul is this ok?

@flamerohr
Copy link
Contributor

I'm in favour of @tractorcow's suggestion.
Removing the count when there's no title seems like unintended loss of data, title shouldn't be linked to the count in that way.

If it was a clean "remove count from the top", then I'd agree with that, since the count is also available at the bottom of the gridfield

@tractorcow
Copy link
Contributor

@clarkepaul can you please comment and let us know if this is ok. I can make the changes if so.

@clarkepaul
Copy link
Contributor Author

clarkepaul commented Jan 22, 2018

Good idea there, although I'm not sure what actions will end up in that place (right side on gridfields). I know the "link to existing records" action lives there but I want to remove that interaction to be coupled with the "Add" action or at least next to it as a smaller action.

Some of my reasoning for not needing that information is that the gridfields are small enough for most screens to be able to see the bottom with pagination and same info at the bottom, and this info at the top is a factor from preventing people from accessing the page info & pagination at the bottom which is more important.

I guess we can place the "link existing" action to the left of this data for now, can you just align the bottom of the text with the actions on the left please. Happy to proceed with change if others can't think of any other actions on the right side.

@tractorcow
Copy link
Contributor

Link to existing records is a row below that, I think.

@tractorcow
Copy link
Contributor

Oh, it's right.
But, I can make it conditional... if you have a title, page count will go beside it. If you don't have a title, it'll go in the button row.

@tractorcow
Copy link
Contributor

New fix at #403

@flamerohr flamerohr self-assigned this Jan 22, 2018
@flamerohr flamerohr modified the milestones: Recipe 4.0.2, Recipe 4.1.0 Jan 22, 2018
tractorcow pushed a commit to open-sausages/silverstripe-admin that referenced this issue Jan 23, 2018
@elvinas-liut
Copy link

Sorry for resurecting this issue, but the fix did not solve the main problem - unwanted space.

I've attached a screenshot with highlighted empty toolbar header (on 4.1.0-rc2):
untitled

I think, you should have to remove GridFieldToolbarHeader itself to get rid of that empty space.

Also, the fix addesses only the SecurityAdmin. It would be nice to have this in ModelAdmin too.

@dhensby dhensby reopened this Feb 19, 2018
@chillu chillu removed this from the Recipe 4.1.0 milestone Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants