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

Topics' filtration #405

Merged
merged 18 commits into from
May 12, 2021
Merged

Topics' filtration #405

merged 18 commits into from
May 12, 2021

Conversation

GneyHabub
Copy link
Contributor

Implemented search bar and order by column
image

@GneyHabub GneyHabub linked an issue Apr 29, 2021 that may be closed by this pull request
@GneyHabub GneyHabub marked this pull request as ready for review May 1, 2021 09:03
@GneyHabub GneyHabub requested a review from marat-ad May 1, 2021 09:03
Comment on lines 62 to 69
const handleSearch = (e: React.ChangeEvent<HTMLInputElement>) => {
if (debounceTimeout) window.clearTimeout(debounceTimeout);
debounceTimeout = setTimeout(() => setSearch(e.target.value), 300);
};

React.useEffect(() => () => {
if (debounceTimeout) window.clearTimeout(debounceTimeout);
});
Copy link
Member

Choose a reason for hiding this comment

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

You can use useDebouncedCallback (there's an example in Messages component).

className="input"
type="text"
placeholder="Search by Topic Name"
onChange={(e) => handleSearch(e)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onChange={(e) => handleSearch(e)}
onChange={handleSearch}

@@ -41,25 +43,39 @@ const List: React.FC<Props> = ({
const { isReadOnly } = React.useContext(ClusterContext);
const { clusterName } = useParams<{ clusterName: ClusterName }>();
const { page, perPage } = usePagination();
const [orderBy, setOrderBy] = React.useState<
TopicColumnsToSort | undefined
Copy link
Member

Choose a reason for hiding this comment

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

It's better to use null instead of undefined for the state. That way we will know that we intentionally set it to null when we want to reset the value. In case of undefined it's not clear if we set it or it's broken somewhere.

Comment on lines 49 to 65
<th
className={cx(
'is-clickable',
orderBy === TopicColumnsToSort.OUT_OF_SYNC_REPLICAS &&
'has-text-link-dark'
)}
onClick={() => setOrderBy(TopicColumnsToSort.OUT_OF_SYNC_REPLICAS)}
>
Out of sync replicas{' '}
<span className="icon is-small">
{orderBy === TopicColumnsToSort.OUT_OF_SYNC_REPLICAS ? (
<i className="fas fa-caret-up" />
) : (
<i className="fas fa-caret-down" />
)}
</span>
</th>
Copy link
Member

Choose a reason for hiding this comment

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

These th blocks look very similar, could be extracted into own component.

Comment on lines 25 to 29
{orderBy === TopicColumnsToSort.NAME ? (
<i className="fas fa-caret-up" />
) : (
<i className="fas fa-caret-down" />
)}
Copy link
Member

Choose a reason for hiding this comment

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

Usually caret-up indicates that data is sorted in ascending order, and caret-down - in descending. You can use fa-sort icon to indicate that column is sortable.

@marat-ad marat-ad self-requested a review May 3, 2021 10:46
Copy link
Member

@workshur workshur left a comment

Choose a reason for hiding this comment

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

Good progress!

kafka-ui-react-app/src/components/Topics/List/List.tsx Outdated Show resolved Hide resolved
kafka-ui-react-app/src/components/Topics/List/List.tsx Outdated Show resolved Hide resolved
kafka-ui-react-app/src/components/Topics/List/List.tsx Outdated Show resolved Hide resolved
@GneyHabub GneyHabub requested a review from workshur May 6, 2021 14:01
<Search
handleSearch={handleSearch}
placeholder="Search by Topic Name"
/>
Copy link
Member

Choose a reason for hiding this comment

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

we should be able to show current value of search query

interface SearchProps {
handleSearch: (value: string) => void;
placeholder: string;
}
Copy link
Member

Choose a reason for hiding this comment

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

please add initialValue

@GneyHabub GneyHabub requested a review from workshur May 12, 2021 07:43
@GneyHabub GneyHabub merged commit 98fcc90 into master May 12, 2021
@GneyHabub GneyHabub deleted the feature/topics-filter branch May 12, 2021 13:34
@sonarcloud
Copy link

sonarcloud bot commented May 12, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

80.3% 80.3% Coverage
0.0% 0.0% Duplication

javalover123 pushed a commit to javalover123/kafka-ui that referenced this pull request Dec 7, 2022
* Implement topics' filtration
Donutellko pushed a commit to Donutellko/kafka-ui that referenced this pull request Jul 12, 2024
Co-authored-by: iliax <ilya.kuramshin@almatech.dev>
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.

Topic list : add filter
3 participants