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

Ts filters #624

Merged
merged 20 commits into from
Jun 21, 2024
Merged

Ts filters #624

merged 20 commits into from
Jun 21, 2024

Conversation

gustavo-salazar
Copy link
Member

Typescript and Visual framework migration for the browse page filters.

  • EntryListFilters/*
  • ProteinListFilters/*
  • StructureListFilters/*
  • FiltersPanel/*
  • EntryListFilters/*
  • MemberDBSelector

@coveralls
Copy link

Coverage Status

coverage: 21.565% (-0.2%) from 21.8%
when pulling 3b80a42 on ts-filters
into 822af0a on dev.

@coveralls
Copy link

Coverage Status

coverage: 21.565% (-0.2%) from 21.8%
when pulling 3b80a42 on ts-filters
into 822af0a on dev.

@matthiasblum matthiasblum requested a review from tgrego June 13, 2024 07:58
Copy link

@tgrego tgrego left a comment

Choose a reason for hiding this comment

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

Looks good, everything seems to work fine.
The color bubble around the db size on the db selector used to have a padding that is now gone ( bubble covers exactly the text now). It's just visual and works as expected but maybe looked better before... Tried to change it back but without success!

@coveralls
Copy link

Coverage Status

coverage: 21.565% (-0.2%) from 21.8%
when pulling c0b9776 on ts-filters
into 822af0a on dev.

@tgrego
Copy link

tgrego commented Jun 18, 2024

Visually as before now, thanks!

Copy link
Contributor

@matthiasblum matthiasblum left a comment

Choose a reason for hiding this comment

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

Looks good to me.

One thing I noted is that the filters panel is narrower on the structures page. This is not introduced by this PR (already on the live website), but causes longer option labels (e.g. CATH-Gene3D, Cryo-EM) to wrap. Can the panel have a minimum width?

Comment on lines 30 to 52
<label className={css('radio-btn-label', { checked })}>
<input
type="radio"
name="entry_type"
value={taxId}
disabled={isStale}
className={css('radio-btn')}
onChange={onChange}
checked={checked}
style={{ margin: '0.25em' }}
/>
{taxId === 'ALL' ? <div>All</div> : title}
{typeof count === 'undefined' || isNaN(count) ? null : (
<NumberComponent
label
loading={loading}
className={css('filter-label')}
abbr
>
{count}
</NumberComponent>
)}
</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

When hovering the options, there is an animation on the counter instead of the label, unlike other filters.

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch! The css rule for it requires the text of the option(line 41) to be in a <span> tag.
Fix included in the latest commit

@gustavo-salazar
Copy link
Member Author

One thing I noted is that the filters panel is narrower on the structures page. This is not introduced by this PR (already on the live website), but causes longer option labels (e.g. CATH-Gene3D, Cryo-EM) to wrap. Can the panel have a minimum width?

I made the text for the member DB name white-space: nowrap; this way I won't have to guess a minimum width and the panel won't ever be narrower than the longest member DB name.

@coveralls
Copy link

Coverage Status

coverage: 21.565% (-0.2%) from 21.8%
when pulling 6d46e77 on ts-filters
into 822af0a on dev.

@gustavo-salazar gustavo-salazar merged commit 26261fe into dev Jun 21, 2024
3 checks passed
@matthiasblum matthiasblum deleted the ts-filters branch August 2, 2024 13:01
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.

4 participants