kaxil commented on code in PR #70190:
URL: https://github.com/apache/airflow/pull/70190#discussion_r3682598643
##########
registry/src/js/provider-detail.js:
##########
@@ -155,6 +157,44 @@
});
});
+ // "More" overflow menu for module tabs that don't fit in the visible row.
+ var moduleTabMoreBtn = document.getElementById('module-tab-more-btn');
+ var moduleTabMoreMenu = document.getElementById('module-tab-more-menu');
+
+ if (moduleTabMoreBtn && moduleTabMoreMenu) {
+ function closeMoreMenu() {
+ moduleTabMoreBtn.setAttribute('aria-expanded', 'false');
+ moduleTabMoreMenu.hidden = true;
+ }
+
+ moduleTabMoreBtn.addEventListener('click', function(e) {
+ e.stopPropagation();
+ var isOpen = moduleTabMoreBtn.getAttribute('aria-expanded') === 'true';
+ if (isOpen) {
+ closeMoreMenu();
+ } else {
+ moduleTabMoreBtn.setAttribute('aria-expanded', 'true');
+ moduleTabMoreMenu.hidden = false;
+ }
+ });
+
+ document.addEventListener('click', function(e) {
+ if (!moduleTabMoreMenu.contains(e.target) && e.target !==
moduleTabMoreBtn) {
+ closeMoreMenu();
+ }
+ });
+
+ document.addEventListener('keydown', function(e) {
+ if (e.key === 'Escape') closeMoreMenu();
+ });
+
+ // Selecting a tab inside the menu also closes it (filtering itself is
+ // handled by the shared moduleTabs click listener above).
+ moduleTabMoreMenu.querySelectorAll('.module-tab').forEach(function(tab) {
+ tab.addEventListener('click', closeMoreMenu);
Review Comment:
The shared `moduleTabs` listener does `moduleTabs.forEach(t =>
t.classList.remove('active'))` and then marks the clicked tab, and the tab
clicked here is inside the menu that just got hidden. So after picking Dialects
from More, no visible tab is highlighted (All lost `active` too) and there's
nothing showing what the list is filtered by. Adding `active` to the More
button when the selected type is in `overflowTypes` would cover it.
##########
registry/src/_data/types.json:
##########
@@ -58,5 +58,35 @@
"id": "toolset",
"label": "Toolsets",
"icon": "U"
+ },
+ {
+ "id": "extra_link",
Review Comment:
Each entry here feeds more CSS than `.tab-icon.<id>`. `_data/statsData.js`
builds one stats card per types.json entry with `colorClass: t.id` and no
zero-count filter, so `/stats/` needs `.type-icon.<id>` and `.share-bar.<id>`;
`provider-card.njk`'s breakdown bar needs `.provider-card .modules .<id>`. All
three families stop at `decorator` in main.css, so the six new ids get
transparent slices in the listing-page breakdown bar (`.breakdown > div` has no
default background, though widths still sum to 100% and tooltips still work),
six 2.5rem chips on `/stats/` with no background or icon colour, and gray
share-bars since `--meter-color` falls back to `--color-gray-500`.
The `--color-*` tokens are already added in tokens.css, so it's one line per
id in each of the three blocks. `toolset` has the same gap from #70122.
##########
dev/registry/registry_tools/types.py:
##########
@@ -139,11 +181,27 @@
# Class-level sections used by extract_parameters.py (subset of flat that
# list full class paths rather than simple entries).
+#
+# "plugins" and "dialects" are also flat/class-path sections, but each entry
is a
+# dict (plugin-class / dialect-class-name) rather than a bare string, so they
are
+# handled by their own loops in extract_parameters.py instead of this generic
one.
CLASS_LEVEL_SECTIONS: dict[str, str] = {
"notifications": "notifier",
"secrets-backends": "secret",
"logging": "logging",
"executors": "executor",
+ "extra-links": "extra_link",
+ "queues": "queue",
Review Comment:
Worth a follow-up outside this PR: `provider.yaml.schema.json` declares
`queues` items as `{name, message-queue-class}`, the same shape as `plugins`,
but all six providers that set it list a bare class path, which is what this
treats them as. The `items` block has no `type` key, so neither shape is
actually validated. Since this is the first consumer of the section, tightening
it to `items: {type: string}` would stop a future provider following the
documented dict shape from being silently dropped by the
`isinstance(class_path, str)` guard in extract_parameters.py.
##########
dev/registry/extract_versions.py:
##########
@@ -300,15 +320,13 @@ def process_module(module_path: str, module_type: str,
integration: str, categor
if mp:
process_module(mp, "transfer", source, get_category(source))
- # Class-level sections (full class paths, no source file parsing needed)
- FQCN_SECTIONS: dict[str, tuple[str, str, str]] = {
- # yaml_key: (module_type, category, description_suffix)
- "notifications": ("notifier", "notifications", "notifier"),
- "secrets-backends": ("secret", "secrets", "secrets backend"),
- "logging": ("logging", "logging", "log handler"),
- "executors": ("executor", "executors", "executor"),
- }
- for yaml_key, (mod_type, category, desc_suffix) in FQCN_SECTIONS.items():
+ # Class-level sections (full class paths, no source file parsing needed).
+ # Iterating CLASS_LEVEL_SECTIONS keeps this in sync with the single
+ # source of truth in types.py; a section added there without a matching
+ # FQCN_DESC_SUFFIXES entry raises KeyError instead of silently skipping.
+ for yaml_key, mod_type in CLASS_LEVEL_SECTIONS.items():
Review Comment:
Good that this tracks `CLASS_LEVEL_SECTIONS` now, but `plugins` and
`dialects` are deliberately excluded from that dict (dict-shaped entries), and
they only get their own loops in `extract_parameters.py`. So version pages
built here still miss those two types while the latest page has them: postgres,
common.sql and microsoft.mssql get a Dialects tab on latest and nothing on
older versions, same for the seven providers with `plugins` (databricks, edge3,
informatica, openlineage, apache.hive, apache.kafka, common.ai).
`test_all_class_level_sections_produce_covered_types` won't catch it either,
since it iterates the same dict.
##########
registry/src/js/provider-detail.js:
##########
@@ -155,6 +157,44 @@
});
});
+ // "More" overflow menu for module tabs that don't fit in the visible row.
+ var moduleTabMoreBtn = document.getElementById('module-tab-more-btn');
+ var moduleTabMoreMenu = document.getElementById('module-tab-more-menu');
+
+ if (moduleTabMoreBtn && moduleTabMoreMenu) {
+ function closeMoreMenu() {
+ moduleTabMoreBtn.setAttribute('aria-expanded', 'false');
+ moduleTabMoreMenu.hidden = true;
+ }
+
+ moduleTabMoreBtn.addEventListener('click', function(e) {
+ e.stopPropagation();
+ var isOpen = moduleTabMoreBtn.getAttribute('aria-expanded') === 'true';
+ if (isOpen) {
+ closeMoreMenu();
+ } else {
+ moduleTabMoreBtn.setAttribute('aria-expanded', 'true');
+ moduleTabMoreMenu.hidden = false;
+ }
+ });
+
+ document.addEventListener('click', function(e) {
+ if (!moduleTabMoreMenu.contains(e.target) && e.target !==
moduleTabMoreBtn) {
+ closeMoreMenu();
+ }
+ });
+
+ document.addEventListener('keydown', function(e) {
+ if (e.key === 'Escape') closeMoreMenu();
Review Comment:
If focus is inside the menu when Escape fires, setting `hidden` drops focus
to `<body>`. A `moduleTabMoreBtn.focus()` inside `closeMoreMenu` keeps it on
the trigger. Related: `role="menu"` / `role="menuitem"` implies arrow-key
navigation, which isn't wired up, so dropping the roles may be the easier
promise to keep.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
