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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tags on cards for people section #5765

Merged
merged 3 commits into from May 29, 2019

Conversation

@CleverFool77
Copy link
Member

commented May 22, 2019

Fixes #5707 part

Screenshot from 2019-05-22 15-47-45
Screenshot from 2019-05-22 15-48-52

  • PR is descriptively titled 馃搼 and links the original issue above 馃敆
  • tests pass -- look for a green checkbox 鉁旓笍 a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 馃搧
  • screenshots/GIFs are attached 馃搸 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

@CleverFool77 CleverFool77 requested a review from gautamig54 May 22, 2019

@CleverFool77 CleverFool77 changed the title [WIP] tags on cards for people section tags on cards for people section May 22, 2019

@CleverFool77 CleverFool77 changed the title tags on cards for people section Add tags on cards for people section May 22, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 23, 2019

Hi @jywarren @gauravano I wanted to know that Is the method I followed for this is correct ?
I need bit of help here.
Thanks !!

@jywarren

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

I think this is looking good -- do you think you could make the tags a bit more like they appear on https://publiclab.org/tags? This is awesome!

app/views/users/list.html.erb Outdated Show resolved Hide resolved
app/views/users/list.html.erb Outdated Show resolved Hide resolved
@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

I think this is looking good -- do you think you could make the tags a bit more like they appear on https://publiclab.org/tags? This is awesome!

Hi @jywarren The code I'm using for tags UI is same as used in all other ages. It's just due to my screenshot which seems to changes the color to bit purple from blue.

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

I just updated the code and new UI is ;
Screenshot from 2019-05-24 11-42-36

the color looks different but in actual its badge-primary only.
Thanks !!!

@CleverFool77 CleverFool77 force-pushed the CleverFool77:tags-on-cards branch from 18a833a to 82ad4fc May 24, 2019

@jywarren

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

Oh ok! Good to know, thanks! Is this ready for final review and merge? Thank you!

@jywarren

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

Actually one more thing; i think @gautamig54 noticed this but Junction is set as the font for many parts of the page, but as per the Style Guide, we want to only use it for headings like h1, h2, h3. Would you mind adjusting this? Thank you so much!

@CleverFool77 CleverFool77 force-pushed the CleverFool77:tags-on-cards branch from f0683ba to 82ad4fc May 24, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

Hi @jywarren I just fixed the font bug. I removed the assignment of font in beginning for whole section. So now It's working only for headings.
Thanks !!!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

This is ready for merge. I guess.
Thanks !!!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

I'll update few more stuff.
Thanks !!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

I just noticed that the class which helps in stacking map above in smaller screen seems to be removed.
Maybe it was removed by someone while resolving merge conflicts.
I'll add it back then.

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

Hi @jywarren I did the changes online as my local branch commits are mess but I've checked the changes locally.
I guess Now this PR is ready for merge.
Thanks !!

@jywarren

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

Oh cool! Can you upload one more screenshot? (hopefully screenshots won't be as needed soon - see #5320) Just to see how the fonts work? Thank you!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 25, 2019

Oh cool! Can you upload one more screenshot? (hopefully screenshots won't be as needed soon - see #5320) Just to see how the fonts work? Thank you!

Hi @jywarren
Sorry I slept yesterday before this message.
Here is the scrrenshot.
Screenshot from 2019-05-25 14-47-09

Thanks !!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented May 25, 2019

The new feature of having screenshot during system test is awesome.
Damn !!!!!!!!!!!!!!!!!!!!!

@CleverFool77 CleverFool77 requested a review from jywarren May 29, 2019

@jywarren jywarren merged commit e11085b into publiclab:master May 29, 2019

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

UI improvements - Summer Of Code 2019 automation moved this from In progress to Done May 29, 2019

@jywarren

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

Awesome here! 馃槃

@CleverFool77 CleverFool77 deleted the CleverFool77:tags-on-cards branch Jun 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can鈥檛 perform that action at this time.