Jade spector improvements #258
Conversation
1) Changed the URL for the Instagram Page. Points to team's Instagram profile page: https://www.instagram.com/cibmangotree 2) Added a Linkdlin button. Points to: https://www.linkedin.com/company/cib-mango-tree.
Updated description to add linkdlin Url
KristijanArmeni
left a comment
KristijanArmeni
left a comment
There was a problem hiding this comment.
Thanks for opening this PR @Jade-Spector. I made code-specific comments in individual line files. I hope you can see them (mostly about spelling and adding an icon).
Just some general notes:
- Please also fill edit the PR form ("Related issue Nr" etc.), even if it is straightforward now, it's good to document these things such that it stays logged moving forward
- Same for the PR title -- just make sure it spells out the main contribution ("Add LInkedIn log to GUI prototype" etc.)
I re-set the target branch of this PR from main to gui-protoype.
Other than that, it looks great!
| description="Instagram profile URL", | ||
| ) | ||
|
|
||
| linkdlin_url: str = Field( |
There was a problem hiding this comment.
Can we name this variable linkedin_url (instead of linkdlin_url)?
|
|
||
| linkdlin_url: str = Field( | ||
| default="https://www.linkedin.com/company/cib-mango-tree", | ||
| description="Linkdlin profile ", |
There was a problem hiding this comment.
Change description to "LinkedIn profile"
| - Left: License information | ||
| - Center: Project attribution | ||
| - Right: External links (GitHub, Instagram) | ||
| - Right: External links (GitHub, Instagram,Linkdin) |
There was a problem hiding this comment.
"LinkedIn" (so, in string text, let's just spell out the brand as is to remove any ambiguity)
| on_click=lambda: self.navigate_to_external(gui_urls.linkdlin_url), | ||
| ).props("flat round") | ||
| with linkdin_btn: | ||
| linkdin_svg = self._load_svg_icon("instagram") |
There was a problem hiding this comment.
Shouldn't this be loading a LinkedIn logo? It's currently going to load "instagram" logo. To add an icon, you need to find an .svg of an icon, place it in gui/icons/ and modify the _load_csv_icon() method accordingly.
I found insta/github icons at https://simpleicons.org which are licensed by CC0. We should make sure that similar llicense is in place for any linked in icon too.
KristijanArmeni
added
domain: ux
2 participants
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this introduce a breaking change?
Other information