Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Cloud/UI tokens page#16746

Merged
artemruts merged 25 commits intomainfrom
cloud/ui-tokens-page
Jan 7, 2021
Merged

Cloud/UI tokens page#16746
artemruts merged 25 commits intomainfrom
cloud/ui-tokens-page

Conversation

@artemruts
Copy link
Copy Markdown
Contributor

@artemruts artemruts commented Dec 15, 2020

Description

Fixes #16325 Modified tokens page as per new designs in Figma.
User only changes, side admins still have previous UI.

Demo

tokens

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 15, 2020

Codecov Report

Merging #16746 (19adc18) into main (44261af) will decrease coverage by 0.07%.
The diff coverage is 2.85%.

@@            Coverage Diff             @@
##             main   #16746      +/-   ##
==========================================
- Coverage   52.29%   52.21%   -0.08%     
==========================================
  Files        1699     1705       +6     
  Lines       84005    84139     +134     
  Branches     7788     7657     -131     
==========================================
+ Hits        43931    43935       +4     
- Misses      36201    36332     +131     
+ Partials     3873     3872       -1     
Flag Coverage Δ
go 51.40% <ø> (+<0.01%) ⬆️
integration 29.69% <66.66%> (+<0.01%) ⬆️
storybook 28.74% <ø> (ø)
typescript 54.12% <2.85%> (-0.29%) ⬇️
unit 34.85% <0.00%> (-0.19%) ⬇️
Impacted Files Coverage Δ
...ent/web/src/components/externalServices/backend.ts 6.25% <ø> (ø)
...c/components/externalServices/externalServices.tsx 16.25% <ø> (ø)
...rc/user/settings/UserAddCodeHostsPageContainer.tsx 0.00% <0.00%> (ø)
.../settings/codeHosts/AddCodeHostConnectionModal.tsx 0.00% <0.00%> (ø)
...t/web/src/user/settings/codeHosts/CodeHostItem.tsx 0.00% <0.00%> (ø)
...ttings/codeHosts/RemoveCodeHostConnectionModal.tsx 0.00% <0.00%> (ø)
...ttings/codeHosts/UpdateCodeHostConnectionModal.tsx 0.00% <0.00%> (ø)
...c/user/settings/codeHosts/UserAddCodeHostsPage.tsx 0.00% <0.00%> (ø)
...ent/web/src/user/settings/codeHosts/modalHints.tsx 0.00% <0.00%> (ø)
client/web/src/user/settings/sidebaritems.ts 100.00% <ø> (ø)
... and 9 more


import CheckCircleIcon from 'mdi-react/CheckCircleIcon'
import AlertCircleIcon from 'mdi-react/AlertCircleIcon'
import CircleOutlineIcon from 'mdi-react/CircleOutlineIcon'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@quinnkeast I couldn't find the dashed circle in mdi-react. Do you want me to make a custom svg?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, please do! The dashes are an important visual element for making certain it isn't interpreted as a radio button. We'll also use it in other locations as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll make a ticket to do this, I played around but wasn't happy with the TS types for the icon component.

@artemruts artemruts marked this pull request as ready for review December 16, 2020 18:20
@artemruts artemruts requested review from a team, marekweb and tjkandala and removed request for a team December 16, 2020 18:20
@artemruts
Copy link
Copy Markdown
Contributor Author

@tjkandala Was only able to change initial icon color with .mdi-icon class, also found this comment on levrik/mdi-react

Seems like fill can be changed dynamically with :hover { fill } but for default fill we have to use .mdi-icon class 🤷.
There are also ways to override default fill and color with props but I think we want CSS.

Copy link
Copy Markdown
Contributor

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

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

LGTM besides my comments

@felixfbecker
Copy link
Copy Markdown
Contributor

@artemruts the icon should use the current text color for the fill using the currentColor keyword, this is in both MDI React and in the icon-inline class. So you should be able to just change the CSS color property of the icon or of the parent, using either CSS or even better just the text-danger etc helper classes.

@artemruts
Copy link
Copy Markdown
Contributor Author

@artemruts the icon should use the current text color for the fill using the currentColor keyword, this is in both MDI React and in the icon-inline class. So you should be able to just change the CSS color property of the icon or of the parent, using either CSS or even better just the text-danger etc helper classes.

@felixfbecker Thank you, Felix! text helper classes work, great solution!

@artemruts artemruts merged commit c3a5322 into main Jan 7, 2021
@artemruts artemruts deleted the cloud/ui-tokens-page branch January 7, 2021 20:38
@artemruts artemruts linked an issue Jan 11, 2021 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code Hosts: update existing page and add modal for user tokens. Users can create and manage code host connections

4 participants