Skip to content

Search UI - #59

Draft
corovcam wants to merge 12 commits into
rdm-14from
corovcam/fe-430-search-stranka-a-vysledky
Draft

Search UI#59
corovcam wants to merge 12 commits into
rdm-14from
corovcam/fe-430-search-stranka-a-vysledky

Conversation

@corovcam

@corovcam corovcam commented Dec 9, 2025

Copy link
Copy Markdown
Collaborator

Applies theme overrides and adds variables for the RDM (Research Data Management) platform. This includes changes to colors, styles, and layouts for various elements and modules to align with the new design.

The changes include:

  • Adds overrides for collections like tables, grids, and menus.
  • Adds overrides for elements like buttons, labels, and lists.
  • Adds overrides for modules like dropdowns and accordions.
  • Adjusts global styles and variables to ensure consistency across the platform.

These changes improve the overall visual appearance and user experience of the RDM platform.

image

Adds a new search page for datasets with active filters, result options and result list item components.

Includes creatibutor component for handling both persons and organizations.

Includes access status component to display access rights information.

Implements a new layout for search apps and adjust existing search components.
Applies theme overrides and adds variables for the RDM (Research Data Management) platform. This includes changes to colors, styles, and layouts for various elements and modules to align with the new design.

The changes include:
- Adds overrides for collections like tables, grids, and menus.
- Adds overrides for elements like buttons, labels, and lists.
- Adds overrides for modules like dropdowns and accordions.
- Adjusts global styles and variables to ensure consistency across the platform.

These changes improve the overall visual appearance and user experience of the RDM platform.

Related to FE-430
Copilot AI review requested due to automatic review settings December 9, 2025 08:54
@corovcam corovcam changed the title Corovcam/fe-430-search-stranka-a-vysledky Search UI Dec 9, 2025

This comment was marked as outdated.

@corovcam
corovcam marked this pull request as draft December 9, 2025 09:48
Comment thread static/icons/locks/closed_access.svg Outdated
Copilot AI review requested due to automatic review settings December 10, 2025 11:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 49 out of 60 changed files in this pull request and generated 19 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ui/datasets/semantic-ui/js/datasets/search/EmptyResultsElement.jsx Outdated
Comment thread ui/datasets/semantic-ui/js/datasets/search/SortElement.jsx
Comment thread ui/datasets/semantic-ui/js/datasets/search/ResultsListItem.jsx Outdated
}

.svg-container {
flex: 0 1 350px;;

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

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

Extra semicolon after CSS value. Line 21 has flex: 0 1 350px;; with two semicolons. While browsers are generally forgiving, this is invalid CSS syntax and should be fixed:

flex: 0 1 350px;

Copilot uses AI. Check for mistakes.
Comment thread ui/datasets/semantic-ui/js/datasets/search/Creatibutors.jsx
Comment thread ui/datasets/semantic-ui/js/datasets/search/FacetValueElement.jsx
Comment thread ui/datasets/semantic-ui/js/datasets/search/SearchAppLayout.jsx
Comment thread ui/datasets/semantic-ui/js/datasets/search/Creatibutor.jsx
Comment thread ui/datasets/semantic-ui/js/datasets/search/ResultsListItem.jsx Outdated
Comment thread ui/datasets/semantic-ui/js/datasets/search/ResultsListItem.jsx Outdated
@mirekys

mirekys commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

Pls rebase this PR, there are many files deleted that shouldn't be deleted (e.g. .variables, .overrides)

Copilot AI review requested due to automatic review settings December 12, 2025 17:36
display: flex;
flex-direction: column;
position: fixed;
bottom: 148px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use rems or even better vh unit to position on screen

flex-direction: column;
position: fixed;
bottom: 148px;
right: 44px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

}

@media screen and (max-width: 1679px) {
bottom: 90px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here


@media screen and (max-width: 768px) {
right: 25px;
bottom: 148px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here, pixels doesnt work well on devices with various DPI, scaling

}
}

#user-profile-menu.ui.menu {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why this is needed?

Comment thread invenio.cfg

from oarepo_ui.overrides import UIComponentOverride, UIComponent

OAREPO_UI_OVERRIDES = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can leave this here

@@ -0,0 +1,25 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cannot use this graphics

@@ -0,0 +1,96 @@
import React from "react";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not use RDM code for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be available in app-RDM

@corovcam corovcam Dec 15, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

image Because I don't like the way it looks by default (roles are missing). Of course I could copy their code, but that is what I did with my previous NMA UI implementation (I copied and modified nr-docs design). The only thing that changes here since previous NMA logic is the metadata model, but the UI design stays the same/similar. Or since we don't have any usable NMD design, I could of course, copy the whole RDM Search UI, delete all overrides and less and only style colors, etc. (like we did with detail)

Comment thread ui/datasets/semantic-ui/js/datasets/search/EmptyResultsElement.jsx Outdated
@@ -0,0 +1,50 @@
import React from "react";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This might not be needed as discussed today

This comment was marked as outdated.

@@ -0,0 +1,29 @@
import React from "react";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not RDM component?

{i18next.t("Uploaded on")} <span>{createdDate}</span>{" "}
{version && `(${i18next.t("version")}: ${version})`}
</>
{sanitizedDescription && (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sanitized vs truncated?


return (
<Item key={result.id} data-testid="result-item">
<Item className="results-list-item-main">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bring back test id

@@ -1,71 +1,188 @@
import React from "react";
import React, { useContext, useState } from "react";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like taken from previous nma, much simpler would be to copy from RDM result item & adapt it to design as it expects RDM model we use now

const SortLabelMobile = (cmp) => cmp;

export const ResultOptions = ({ currentResultsState = {} }) => {
const { total } = currentResultsState.data;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we 100% sure this cannot happen? Will break whole search ui otherwise

</Accordion>
)}
{orgCreatibutors.length > 0 && (
<Accordion style={personalCreatibutors.length > 0 ? { marginTop: "-1rem" } : {}}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No fixed inline styles, conditionally add rel-mt-1 class instead

);
};

CreatibutorsList.propTypes = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Creatibutors are solved in RDM (uses the same record model as current nma), can be taken & adapted from there

? creatibutor.role.title
: (creatibutor?.role?.title?.["en"] ||
creatibutor?.role?.title?.["cs"] ||
creatibutor?.role?.title_l10n ||

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Title l10n should be the first option (already localized by backend)

return person.name || familyName || givenName || "";
};

const normalizeIdentifier = (identifier) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be handled by backend in ui serialization

@@ -0,0 +1,96 @@
import React from "react";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be available in app-RDM

@corovcam

corovcam commented Dec 16, 2025

Copy link
Copy Markdown
Collaborator Author

To answer previous feedback comments and to continue with this one, I have a general question about the Search UI design. What I've done here is that I coppied everything I deemed necessary from previous NMA along with model changes and removed some unnecessary stuff. But from what I understand from feedback I could also only use RDM components and not do any changes here, like we did with detail.

I made a PoC of Search UI using only RDM component with minor changes. The goal, as I understood from the feedback comments by @mirekys was to minimize code (if that is our objective). The design along with minimized code can be seen here: #88
@mesemus @mirekys @Ducica how do we proceed?

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

3 participants