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

Mixed fixes - from mh's and blake's feedbacks #509

Merged

Conversation

vittoriaThinkst
Copy link
Contributor

@vittoriaThinkst vittoriaThinkst commented Jul 3, 2024

Proposed changes

Home Page

Fix tooltip for Cards
Fix border radius
Remove focus after clicking on modal
Searchbar: reposition icon
Fix page paddings
Remove icons from navbar
Remove 'document' from token's labels

Modal Add Token

Fix Notification inputs by adding custom arrows
Remove 'Notification settings' title
Fix border radius
Add 'required field' hint
Fix required fields for all form that needed it

Screenshot 2024-07-03 at 17 07 46 Screenshot 2024-07-03 at 17 07 17 Screenshot 2024-07-03 at 17 09 13

Minor tweaks

Increment border radius of most elements
Fix some alignments
Fix some wording/typos

Components

Add a new BaseLabel component + tests
Add a new BaseLabelArrow components + tests
Fix tests for BaseInput

id: string;
label: string;
arrowVariant?: 'one' | 'two';
arrowWordPosition?: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

heh, this component is very cute

to over-engineer this even further you could even see this being an array (not really feedback)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't tempt me...!

Comment on lines 27 to 31
<!--- CTA text --->
<div
class="w-full leading-5 font-semibold border-t-2 border-grey-50 text-grey-700 h-[3rem] rounded-b-xl transition duration-100 hover-card shadow-solid-shadow-grey card-button justify-center items-center flex px-8"
class="w-full leading-5 font-semibold border-t-2 border-grey-50 text-grey-700 h-[3rem] rounded-b-2xl transition duration-100 hover-card shadow-solid-shadow-grey card-button justify-center items-center flex px-8"
>
{{ isHoverCard ? 'Create Token' : title }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

i guess this is just github parsing this file weirdly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhm what's weird about it?

@vittoriaThinkst vittoriaThinkst merged commit 45309ed into Feature_branch_New_UI Jul 3, 2024
@vittoriaThinkst vittoriaThinkst deleted the Fix-for-mh-review-blake-review branch July 3, 2024 18:28
mclmax pushed a commit that referenced this pull request Jul 16, 2024
* Add base label, add required label, fix searchbar icon position, fix wording 'how to deploy, make required fields

* make stuff rounder

* Revert Canarytoken setting title, center radio input

* fix padding on required

* add label arrow, fix tests

* remove focus from selected card
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