Skip to content

Do not display HTML links when current search has no results and adding another filter wouldn't add more. #8662

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Do not display HTML links when current search has no results and adding another filter wouldn't add more. #8662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/lib/frontend/templates/listing.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ String renderPkgIndexPage(
packageList: packageList(searchResultPage),
pagination: searchResultPage.hasHit ? paginationNode(links) : null,
openSections: openSections,
mayDisplayRestrictiveHtmlLink: searchResultPage.totalCount > 0,
);

String pageTitle = topPackages;
Expand Down
59 changes: 50 additions & 9 deletions app/lib/frontend/templates/views/pkg/index.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ d.Node packageListingNode({
required d.Node packageList,
required d.Node? pagination,
required Set<String>? openSections,

/// Whether the search form input fields (on the left side) that would create
/// a more restrictive search expression (by adding extra filtering conditions),
/// should have a HTML link displayed. As such links are crawled by both human and
/// bot traffic, we may not want to expose them when their usefulness is zero.
/// We still keep the checkbox and the clickability of the label for human users.
required bool mayDisplayRestrictiveHtmlLink,
}) {
final matchHighlights = [
if (nameMatches != null) nameMatches,
Expand All @@ -40,13 +47,15 @@ d.Node packageListingNode({
searchForm: searchForm,
innerContent: innerContent,
openSections: openSections ?? const <String>{},
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
);
}

d.Node _searchFormContainer({
required SearchForm searchForm,
required d.Node innerContent,
required Set<String> openSections,
required bool mayDisplayRestrictiveHtmlLink,
}) {
return d.div(
classes: [
Expand All @@ -67,31 +76,37 @@ d.Node _searchFormContainer({
platform: PlatformTagValue.android,
label: 'Android',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_platformCheckbox(
platform: PlatformTagValue.ios,
label: 'iOS',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_platformCheckbox(
platform: PlatformTagValue.linux,
label: 'Linux',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_platformCheckbox(
platform: PlatformTagValue.macos,
label: 'macOS',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_platformCheckbox(
platform: PlatformTagValue.web,
label: 'Web',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_platformCheckbox(
platform: PlatformTagValue.windows,
label: 'Windows',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
],
),
Expand All @@ -105,11 +120,13 @@ d.Node _searchFormContainer({
sdk: SdkTagValue.dart,
label: 'Dart',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_sdkCheckbox(
sdk: SdkTagValue.flutter,
label: 'Flutter',
searchForm: searchForm,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
],
),
Expand All @@ -125,6 +142,7 @@ d.Node _searchFormContainer({
label: 'OSI approved',
searchForm: searchForm,
title: 'Show only packages with OSI approved license.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
],
),
Expand All @@ -140,6 +158,7 @@ d.Node _searchFormContainer({
label: 'Flutter Favorite',
searchForm: searchForm,
title: 'Show only Flutter Favorite packages.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_tagBasedCheckbox(
tagPrefix: 'show',
Expand All @@ -148,34 +167,40 @@ d.Node _searchFormContainer({
searchForm: searchForm,
title:
'Show unlisted, discontinued and legacy Dart 1.x packages.',
isPermissiveWhenChecked: true,
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_tagBasedCheckbox(
tagPrefix: 'has',
tagValue: 'screenshot',
label: 'Has screenshot',
searchForm: searchForm,
title: 'Show only packages with screenshots.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_tagBasedCheckbox(
tagPrefix: 'is',
tagValue: 'dart3-compatible',
label: 'Dart 3 compatible',
searchForm: searchForm,
title: 'Show only packages compatible with Dart 3.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_tagBasedCheckbox(
tagPrefix: 'is',
tagValue: 'plugin',
label: 'Flutter plugin',
searchForm: searchForm,
title: 'Show only Flutter plugins.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
_tagBasedCheckbox(
tagPrefix: 'is',
tagValue: 'wasm-ready',
label: 'WASM ready',
searchForm: searchForm,
title: 'Show only WASM ready packages.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
),
],
),
Expand All @@ -193,27 +218,31 @@ d.Node _platformCheckbox({
required String platform,
required String label,
required SearchForm searchForm,
required bool mayDisplayRestrictiveHtmlLink,
}) {
return _tagBasedCheckbox(
tagPrefix: 'platform',
tagValue: platform,
label: label,
searchForm: searchForm,
title: 'Show only packages that support the $label platform.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
);
}

d.Node _sdkCheckbox({
required String sdk,
required String label,
required SearchForm searchForm,
required bool mayDisplayRestrictiveHtmlLink,
}) {
return _tagBasedCheckbox(
tagPrefix: 'sdk',
tagValue: sdk,
label: label,
searchForm: searchForm,
title: 'Show only packages that support the $label SDK.',
mayDisplayRestrictiveHtmlLink: mayDisplayRestrictiveHtmlLink,
);
}

Expand All @@ -223,9 +252,13 @@ d.Node _tagBasedCheckbox({
required String label,
required SearchForm searchForm,
required String title,
bool isPermissiveWhenChecked = false,
required bool mayDisplayRestrictiveHtmlLink,
}) {
final tag = '$tagPrefix:$tagValue';
final toggledSearchForm = searchForm.toggleRequiredTag(tag);
final isChecked = searchForm.parsedQuery.tagsPredicate.isRequiredTag(tag);
final isRestrictiveOnAction = (!isPermissiveWhenChecked) ^ isChecked;
return _formLinkedCheckbox(
id: 'search-form-checkbox-$tagPrefix-$tagValue',
label: label,
Expand All @@ -235,6 +268,7 @@ d.Node _tagBasedCheckbox({
tag: tag,
action: 'filter-$tagPrefix-$tagValue',
title: title,
displayLabelAsLink: !isRestrictiveOnAction || mayDisplayRestrictiveHtmlLink,
);
}

Expand All @@ -247,7 +281,12 @@ d.Node _formLinkedCheckbox({
String? tag,
required String? action,
String? title,
required bool displayLabelAsLink,
}) {
final dataAttributes = {
if (action != null) 'data-action': action,
if (tag != null) 'data-tag': tag,
};
return d.div(
classes: ['search-form-linked-checkbox'],
attributes: {
Expand All @@ -256,15 +295,17 @@ d.Node _formLinkedCheckbox({
child: material.checkbox(
id: id,
label: label,
labelNodeContent: (label) => d.a(
href: toggledSearchForm.toSearchLink(),
text: label,
attributes: {
if (action != null) 'data-action': action,
if (tag != null) 'data-tag': tag,
},
rel: 'nofollow',
),
labelNodeContent: (label) => displayLabelAsLink
? d.a(
href: toggledSearchForm.toSearchLink(),
text: label,
attributes: dataAttributes,
rel: 'nofollow',
)
: d.span(
text: label,
attributes: dataAttributes,
),
checked: isChecked,
indeterminate: isIndeterminate,
),
Expand Down
10 changes: 5 additions & 5 deletions pkg/web_app/lib/src/search.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ void _setEventsForSearchForm() {
// checking the checkbox will trigger a click on the link.
document.querySelectorAll('.search-form-linked-checkbox').forEach((e) {
final checkbox = e.querySelector('input');
final link = e.querySelector('a');
if (checkbox != null && link != null) {
final tag = link.dataset['tag'];
final action = link.dataset['action'];
final linkOrLabel = e.querySelector('[data-tag]');
if (checkbox != null && linkOrLabel != null) {
final tag = linkOrLabel.dataset['tag'];
final action = linkOrLabel.dataset['action'];
if (tag == null) return;

Future<void> handleClick(Event event) async {
Expand All @@ -84,7 +84,7 @@ void _setEventsForSearchForm() {
}

checkbox.onChange.listen(handleClick);
link.onClick.listen(handleClick);
linkOrLabel.onClick.listen(handleClick);
e.onClick.listen(handleClick);
}
});
Expand Down