WIP: Fixed issue #843 #858

alvinche · 2026-09-23T22:14:48Z

alvinche commented

2026-09-23 22:14:48 +00:00

Passed locationFilterQuery to SelectLocation, compared to default state values and added an 'icon-div--active' css class with border, tinted background, and filled icon

Screenshot 2026-09-23 181422.png

8.6 KiB

alvinche added 1 commit

2026-09-23 22:14:48 +00:00

Fixed issue #843

kennel/build skipped: no services or static sites defined

Details

ea4d03704a

Passed locationFilterQuery to SelectLocation, compared to default state values and added an icon-div--active css class with border, tinted background, and filled icon

arom1a requested changes

2026-10-04 04:03:24 +00:00

arom1a left a comment

Short and concise changes for this feature, but the details can be more polished!

apps/web/src/components/SelectDropdown.module.css

					
				@ -21,1 +23,4 @@

				}

				.icon-div--active {

				    border-color: var(--hover-accent-color);

				    background: color-mix(in srgb, var(--red-600) 20%, var(--input-bg));

to be more consistent with the context, i think we can make the first color to blend together to be --hover-accent-color?

to be more consistent with the context, i think we can make the first color to blend together to be `--hover-accent-color`?

apps/web/src/components/SelectDropdown.module.css

					
				@ -22,0 +28,4 @@

				    svg {

				        fill: currentColor;

				    }

I do not really like the filled version. Also, the filled "sorting" icon looks kinda bad. It's a personal choice to fill the filter button or not though. So what do you think?

apps/web/src/components/SelectDropdown.module.css

					
				@ -24,1 +35,3 @@

				    transition: all 0.5s;

				    transition:

				        box-shadow 0.5s,

				        border-color 0.5s;

I think we can just delete all of these to inherit from the non-hover case

apps/web/src/components/SelectLocation.tsx

					
				@ -12,3 +14,3 @@

				}

				function SelectLocation({ setLocationFilterQuery, locations }: SelectLocationProps) {

				function SelectLocation({ locationFilterQuery, setLocationFilterQuery, locations }: SelectLocationProps) {

nice and clean method to detect if an filter is active. my initial thought was more complicated than this!

apps/web/src/components/SelectSort.tsx

					
				@ -19,3 +20,3 @@

				    return (

				        <div className={css.container}>

				            <div className={css['icon-div']}>

				            <div className={clsx(css['icon-div'], sortBy !== 'open' && css['icon-div--active'])}>

this 'open' and '' above for SelectLocation is kinda like magic strings, that they are not self-explanatory that they are the default options . For maintainability, I think we could make export constants like LOCATION_DEFAULT and SORT_DEFAULTto replace these two magic strings

this `'open'` and `''` above for SelectLocation is kinda like magic strings, that they are not self-explanatory that they are the default options . For maintainability, I think we could make export constants like `LOCATION_DEFAULT` and `SORT_DEFAULT`to replace these two magic strings

alvinche changed title from ~~Fixed issue #843~~ to WIP: Fixed issue #843

2026-10-04 04:18:48 +00:00

arom1a commented

2026-10-04 04:20:46 +00:00

Oh I also found this bug. When you active the filter, there is a flick in size of the icon. This is because of the box-sizing: border-box that shrinks the box inwards so that the whole thing don't grow in size. For fix, I think we can make the default state to have a 2px border, but only draw 1px of line when hovered

Oh I also found this bug. When you active the filter, there is a flick in size of the icon. This is because of the `box-sizing: border-box` that shrinks the box inwards so that the whole thing don't grow in size. For fix, I think we can make the default state to have a 2px border, but only draw 1px of line when hovered

eats-flick2.mp4

448 KiB

arom1a commented

2026-10-04 04:23:25 +00:00

For title, I think it is better to name it as the actual feature you are working on, and you can mention which issue the pr fixes in the discussion section of the pr. And for naming of the commit message, you can check https://www.conventionalcommits.org/en/v1.0.0/ for better conventions

For title, I think it is better to name it as the actual feature you are working on, and you can mention which issue the pr fixes in the discussion section of the pr. And for naming of the commit message, you can check <https://www.conventionalcommits.org/en/v1.0.0/> for better conventions

kennel/build skipped: no services or static sites defined

Details

This pull request is marked as a work in progress.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin issue-843-active-filter:issue-843-active-filter

git switch issue-843-active-filter

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

ScottyLabs/eats!858

Rows
Columns