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

Multiple technologies choices #432

Closed
wants to merge 5 commits into from
Closed

Conversation

alexluong
Copy link

@alexluong alexluong commented Jun 12, 2019

resolve #413

Please do go over the code and see if there's any problem. Let me know and I'll try my best to fix it. Thanks!

@moshfeu
Copy link
Member

moshfeu commented Jun 12, 2019

Thanks! I'll review it asap. BTW, is that PR done? Because I saw in the code some TODO's.

Copy link
Member

@moshfeu moshfeu left a comment

Choose a reason for hiding this comment

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

Anyway, I'll put it here:

  1. No x on the tags.
  2. The filter button is not in the right place.
  3. When add tags "a" and then "b" and click on "b" the URL is remaining the same. When click on "a", "b" removed from the URL and "a" is still there.
  4. The autocomplete's list in contains tags which already added

@alexluong
Copy link
Author

Thanks for the review @moshfeu. I'll check it out when I have a chance. Can you clarify on your 4th point though?

@moshfeu
Copy link
Member

moshfeu commented Jun 12, 2019

Sure. Let's say you choosed JavaScript. Now, the option shouldn't be in the options list any more.

Before choosing

Capture+_2019-06-12-23-56-01

After choosing (notice that the "Red" option is not in the list anymore)

Capture+_2019-06-12-23-58-19

@moshfeu moshfeu added the wip Work in progress label Jun 13, 2019
@moshfeu
Copy link
Member

moshfeu commented Jun 18, 2019

Hi, any progress?

@alexluong
Copy link
Author

Hey @moshfeu sorry for the late update. I made some changes here. Let me know what else I can do. Thank you!

@moshfeu
Copy link
Member

moshfeu commented Jul 11, 2019

Looks better! Thanks
But:

  1. It's not completely fit to the design. For example the "Add filter" button is overlap the input. Also, the most left tag is not aligned with the input.
  2. The button "Add filter" doesn't do anything (?)
    Also in mobile

@klubiniecki
Copy link
Contributor

@alexluong Hey! Will you have time to make the fixes that @moshfeu pointed out? Thanks for contribution!

@moshfeu
Copy link
Member

moshfeu commented Sep 9, 2019

@alexluong any update?
Also the components changed since the PR created. Maybe it will be better to close this and create a new one?

@moshfeu
Copy link
Member

moshfeu commented Sep 9, 2019

Following my chat with Alex, I'll close that PR and we'll assign it to someone else.
Hopefully, Alex will get back to us soon :)

@moshfeu moshfeu closed this Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple choises filter
3 participants