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

[Feature][3.5][Ready] Custom headings and titles #1527

Merged
merged 6 commits into from Nov 8, 2018

Conversation

tabacitu
Copy link
Member

@tabacitu tabacitu commented Jul 10, 2018

Adds the ability to change what the title, heading and subheading of each CRUD page says.

  • title = what shows up in the browser tab (meta title)
  • heading = biggest text on page (h1)
  • subheading = smaller text next to the biggest text on page

The second parameter is the controller method where it will be used. The action.

Example:

$this->crud->setTitle('NEW ENTRY', 'create');

$this->crud->setHeading('Invoices', 'show');
$this->crud->setSubheading('Showing the details of an invoice', 'show');

$this->crud->setHeading('Monsters', 'index');
$this->crud->setSubheading("A huge entity which makes no sense, that we've created so you can see a lot of features", 'index');

There are a few differences from #1347

  • added Heading customization (can include HTML)
  • added Subheading customization (can include HTML)
  • single API call for each of the customizations (no more setShowTitle), which on the one hand makes it easier to remember and allows the same functionality on future Operations, on the other hand is less sexy;
  • connected the title to the Current Controller Method (the ACTION); that's what's used as a second parameter; this might not be as intuitive as one wants, since you do have to know the controller method where it's used... but it does allow us to use the same feature in all future actions too;

My reservations regarding this PR are:

  1. I'm not really sure a $fallback is needed as parameter. We mostly use that in the views. But maybe plain old null-coalescing operator $crud->getTitle() ?? 'Some fallback' would have been easier to read than $crud->getTitle('Some fallback');
  2. Not sure about using the Action as a second parameter; it's definitely more future-proof; and customizable; it allows people to use it for their own controller methods; but... it does require people to know what controller method is being used:
  • for create, it's create
  • for update, it's edit
  • for preview, it's show
  • for listing, it's index
  • for revisions, it's listRevisions
  • for reorder, it's reorder

Thoughts? @AbbyJanke ?

@tabacitu
Copy link
Member Author

Ok so I removed the $fallback - I was right, much easier to read like this :-) Don't you just love it when you're right? :-))

Would love some feedback on using ACTION as the second parameter - what you guys think, if it's too cryptic or it's actually a good thing because it makes people look at the underlying code. If it's good because it makes it easier to understand WHERE it happens or bad because it prevents us from changing the method names in the future:

  • for create, it's create
  • for update, it's edit
  • for preview, it's show
  • for listing, it's index
  • for revisions, it's listRevisions
  • for reorder, it's reorder

A sexier option would be to use the OPERATION name, but this feature will only be introduced in 3.5. Plus, what if an operation has multiple methods with views? You'd need to customize each one to a different thing and you couldn't do that by specifying the operation - you'll still need to specify the controller method.

@tswonke
Copy link
Contributor

tswonke commented Jul 11, 2018

perfect to merge this with #1504 ;-) :-D

@tabacitu
Copy link
Member Author

@tswonke agreed!

@tswonke
Copy link
Contributor

tswonke commented Jul 11, 2018

what about allowing an array for actions for some cases?

e. g. $this->crud->setTitle('ENTRY', ['create', 'edit']);

@tswonke
Copy link
Contributor

tswonke commented Jul 11, 2018

As long as available actions are commented in the DocBlocks and are as close as possible to existing (e. g. access), I think they are ok.

In the future (4.0...?) one can think about something more "change-stable"...

@tabacitu tabacitu changed the base branch from master to release-3-5 November 8, 2018 08:01
@tabacitu tabacitu changed the title [Feature][3.4][Ready] Custom headings and titles [Feature][3.5][Ready] Custom headings and titles Nov 8, 2018
@tabacitu tabacitu merged commit 382a632 into release-3-5 Nov 8, 2018
@tabacitu tabacitu deleted the custom-headings-and-titles branch November 8, 2018 08:13
@AbbyJanke
Copy link
Contributor

@tabacitu Was going through all my PRs and to pull some files i had commited before but not merged yet, and noticed that we merged custom headings and title but there is no documentation that i can find on it for the website. was going to commit an edit for the docs but couldn't decide where to put it.

TLDR; need documentation for this.

@tabacitu
Copy link
Member Author

@AbbyJanke I added the docs a while back, I don't know if it was before/after your post though... Here they are, under Operations - https://backpackforlaravel.com/docs/3.6/crud-operations#titles-headings-and-subheadings

In hindsight, maybe we should put links to this in other places, maybe in each operation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants