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

Update home page "browse by" to display facet as label if there is only one #8426

Merged

Conversation

tylerjmchugh
Copy link
Contributor

When there are only two facets displayed on the home page the first facet appears as a button. This can be confusing to the users as it appears like there is some action to be taken on this button but clicking it results in no action.

Before changes:

Two facets:

image

Three facets:

image

This PR aims to fix this issue by displaying the facet in place of the "Browse by" label when there is only a single facet to be displayed. The logic checks for two facets however as the last facet is displayed separately.

After changes:

Two facets:

image

Three facets:

image

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

@ianwallen ianwallen added this to the 4.2.11 milestone Oct 11, 2024
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way that the facet panel works is a bit strange, the last item is displayed in a separate row, while all the others in the same row with a button.

I tried these tests with the home facets:

  • 3 facets: works fine. The first 2 are displayed with the buttons to switch them.
  • 2 facets: works fine: The first doesn't have the button, just a label.
  • 1 facet: it doesn't show the label.
    1-facet

@tylerjmchugh
Copy link
Contributor Author

tylerjmchugh commented Oct 15, 2024

The way that the facet panel works is a bit strange, the last item is displayed in a separate row, while all the others in the same row with a button.

I tried these tests with the home facets:

  • 3 facets: works fine. The first 2 are displayed with the buttons to switch them.
  • 2 facets: works fine: The first doesn't have the button, just a label.
  • 1 facet: it doesn't show the label.
    1-facet

I too find it odd that the last facet is shown that way. I had assumed that the label would display similarly to the last facet when there was only a single facet as it was technically the last facet. But I see now that before my changes a single facet is also displayed as "radio buttons".

I just updated the branch to display the label when there is only a single facet as well.

image

Copy link
Contributor

@MichelGabriel MichelGabriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works as expected. Thanks.

@josegar74 josegar74 modified the milestones: 4.2.11, 4.4.7 Oct 16, 2024
@josegar74
Copy link
Member

For doesn't work with 1 facet:

{
  "resourceType": {
    "terms": {
      "field": "resourceType",
      "size": 10
    },
    "meta": {
      "decorator": {
        "type": "icon",
        "prefix": "fa fa-2x pull-left gn-icon-"
      }
    }
  }
}

home-facets-no-label

@tylerjmchugh
Copy link
Contributor Author

For doesn't work with 1 facet:

{
  "resourceType": {
    "terms": {
      "field": "resourceType",
      "size": 10
    },
    "meta": {
      "decorator": {
        "type": "icon",
        "prefix": "fa fa-2x pull-left gn-icon-"
      }
    }
  }
}

home-facets-no-label

@josegar74 Should be fixed now. I realised I only tested the single facet case on 4.2 which did not have the length > 1 check.

Tested with 1, 2, and 3 facets on main and 4.2 and it seems good now.

@ianwallen ianwallen requested a review from josegar74 October 16, 2024 14:50
@josegar74 josegar74 modified the milestones: 4.4.7, 4.4.6 Oct 17, 2024
@josegar74 josegar74 merged commit 7d97656 into geonetwork:main Oct 17, 2024
6 checks passed
@geonetworkbuild
Copy link
Collaborator

The backport to 4.2.x failed:

The process '/usr/bin/git' failed with exit code 1
stderr
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'

stdout
Auto-merging web-ui/src/main/resources/catalog/views/default/templates/home.html
[backport-8426-to-4.2.x 8a8065728e] Update home page to display facet as label if there is only one
 Author: tylerjmchugh <[email protected]>
 Date: Fri Oct 11 14:08:19 2024 -0400
 1 file changed, 5 insertions(+), 2 deletions(-)
Auto-merging web-ui/src/main/resources/catalog/views/default/templates/home.html
[backport-8426-to-4.2.x 7c98561959] Show label with a single facet
 Author: tylerjmchugh <[email protected]>
 Date: Tue Oct 15 09:17:25 2024 -0400
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging web-ui/src/main/resources/catalog/views/default/templates/home.html
On branch backport-8426-to-4.2.x
You are currently cherry-picking commit 38304cada8.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.2.x 4.2.x
# Navigate to the new working tree
cd .worktrees/backport-4.2.x
# Create a new branch
git switch --create backport-8426-to-4.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick cab853f107da7955435ddd8e956ffcc0f1014ccb,7e6fb1d259c58b1765d23a8913bdbf686fb06343,38304cada8122811f1a7e0f8bc9ac087acd421d2
# Push it to GitHub
git push --set-upstream origin backport-8426-to-4.2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.2.x

Then, create a pull request where the base branch is 4.2.x and the compare/head branch is backport-8426-to-4.2.x.

tylerjmchugh added a commit to tylerjmchugh/core-geonetwork that referenced this pull request Oct 17, 2024
…ly one (geonetwork#8426)

* Update home page to display facet as label if there is only one

* Show label with a single facet

* Fix bug with single facet
ianwallen pushed a commit that referenced this pull request Oct 17, 2024
…ly one (#8426) (#8449)

* Update home page to display facet as label if there is only one

* Show label with a single facet

* Fix bug with single facet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants