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

Error variant for Panel #69

Merged
merged 4 commits into from
Jul 21, 2022
Merged

Error variant for Panel #69

merged 4 commits into from
Jul 21, 2022

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Jul 20, 2022

Description

Adding an error variant for the Panel component. Needed for the linked issue.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@olemartinorg
Copy link
Contributor Author

Marked this draft because I'm still not sure if this is where we want it to be. @haakemon Could you take a look at this and tell me how strict we want to be? Right now I'm using --colors-red-300, which is the right color, but it should have a better alias in figma (which I don't know how to fix). Also, we're missing an icon for both the warning and error variants.

Should we (1) wait for the proper alias and icon for some weeks until the relevant people is back from their holiday, or (2) remove the TODOs from the code, create an issue to track these needs, and continue on a faster track to getting this PR ready for merging:

@haakemon
Copy link
Contributor

haakemon commented Jul 21, 2022

I think option 2 is best - no problem to use the generic variables until specific ones are created. As for the icons, there has been some back and forth how they should look in the different variants, so we should tackle that separately anyway :)

Should add a test for the error icon as well

@olemartinorg
Copy link
Contributor Author

Created new issues:

Removed TODOs from the code. Also pushed the missing unit test (sorry, I added that yesterday but forgot to push it! 😆)

@olemartinorg olemartinorg marked this pull request as ready for review July 21, 2022 08:47
Copy link
Contributor

@haakemon haakemon left a comment

Choose a reason for hiding this comment

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

Good stuff!

@haakemon haakemon merged commit 2afb824 into main Jul 21, 2022
@haakemon haakemon deleted the feat/panel-error branch July 21, 2022 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants