-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix: Added Warning and Error icons to Worlflow settings component #4413
Merged
ainouzgali
merged 20 commits into
novuhq:next
from
rayy40:fix/add-icons-to-alert-component
Oct 22, 2023
Merged
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
c32f6c9
fix: Added Warning and Error icons to Worlflow settings component
rayy40 ed9ee69
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 83b9a7c
Merge branch 'next' into fix/add-icons-to-alert-component
davidsoderberg e5607f6
fix: Added the text color of warning and fixed the color of Warning Icon
rayy40 cb66538
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 40e1fa4
fix: Changed the color of warning color prop
rayy40 dfd7feb
Merge branch 'fix/add-icons-to-alert-component' of https://github.com…
rayy40 b4343d3
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 19d4462
fix: Reverted back to the default color for warning color
rayy40 fb6b9f9
Merge branch 'fix/add-icons-to-alert-component' of https://github.com…
rayy40 42a433b
fix: WarningIcon not showing color in other areas
rayy40 b352cc6
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 19e4bff
Merge branch 'next' into fix/add-icons-to-alert-component
LetItRock daa3d9a
fix: Updated some changes for the ux
rayy40 d345560
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 7c7d818
fix: Arrow icon at the end of row and Warning alert being shown at th…
rayy40 ca27b0c
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 a12f61a
fix: Remove duplicate function
rayy40 abfa50a
Merge branch 'next' into fix/add-icons-to-alert-component
rayy40 1d7aee8
Merge branch 'next' into fix/add-icons-to-alert-component
ainouzgali File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct color should be passed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I believe this arrow will be shown immediately after the text, and will not at the end of the row like it should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When placing it just after the text, I need to add another flex container to align the items, and I feel like it's redundant since there is already a flex container
Group
which is a parent forAlertIcons
andText
, so I just addedCircleArrowRight
there too.Please let me know if I am missing something or need to go with a different approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rayy40, in the comment below I added a screenshot 0f your result, as you can see the arrow is not at the end of the row :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ainouzgali , Ah sorry. I wasn't able to understand you before ;(