More flexibility with FAB menu links #13903
Conversation
boring-cyborg
bot
added
area:plugins
area:webserver
|
Hi @jedcunningham! Thanks for adding this with solid looking tests. Just to make this easier to review, would it be possible to add an image of a top-level link and one with a label? Out of curiosity, is there a specific use case you have that caused you to suggest these changes? |
|
@jhtimmins I recently added an extra link to Docs and found it odd that I needed to provide a category_icon, so I went and looked at FAB add_link upstream and saw that it's unused for existing categories anyways. I also noticed one couldn't create a top-level link if they wanted one, so I figured it was worth a PR. appbuilder_menu_items = [
{"name": "Hello", "category": "Docs", "href": "https://google.com",},
{
"name": "labelled",
"label": "Hello (label)",
"category": "Docs",
"href": "https://google.com",
},
{"name": "Top Hello", "href": "https://google.com",},
]Leads to: |
|
Thanks for explaining @jedcunningham. I think this makes a lot of sense and is a good update. @kaxil this code is good to go. |
| @@ -488,7 +488,7 @@ def prepare_dagruns(self): | |||
| ) | |||
|
|
|||
| def test_index(self): | |||
| with assert_queries_count(42): | |||
| with assert_queries_count(43): | |||
There was a problem hiding this comment.
Any idea what caused an extra query?
There was a problem hiding this comment.
It's the new menu link I added (based on me adding a second new link and it going up to 44). I don't know why FAB is doing a query per menu link, but that seems to be the behavior 🤷♂️.
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
bot
added
the
full tests needed
Airflow can be more flexible with the links plugins are allowed to add. Currently, you cannot add a top level link, a link with a label, or even without providing a category_icon (which isn't used anyways). This PR gives plugin authors the flexibility to add any link FAB supports. (cherry picked from commit ef0c17b)
Airflow can be more flexible with the links plugins are allowed to add. Currently, you cannot add a top level link, a link with a label, or even without providing a category_icon (which isn't used anyways).
This PR gives plugin authors the flexibility to add any link FAB supports.