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.

Sign up for GitHub

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

Jump to bottom

feat: redesign 'add text' modal #1117

Merged
merged 32 commits into from
Apr 5, 2024
Merged

feat: redesign 'add text' modal #1117

merged 32 commits into from
Apr 5, 2024

Conversation

ztlee042
Copy link
Contributor

@ztlee042 ztlee042 commented Mar 27, 2024 •
edited
Loading

#1096

2024-03-28
CI tests failed. Maybe due to SDK changes.

@ztlee042 ztlee042 self-assigned this Mar 27, 2024
@ztlee042 ztlee042 marked this pull request as ready for review March 28, 2024 15:43
@ztlee042 ztlee042 requested a review from spaenleh April 1, 2024 10:57
spaenleh
spaenleh requested changes Apr 3, 2024
View reviewed changes
Copy link
Member

@spaenleh spaenleh left a comment

Choose a reason for hiding this comment

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

Looks good, I have a few comments, but overall I think it is very good !

src/components/item/form/DisplayNameForm.tsx Outdated Show resolved Hide resolved
src/components/item/form/DocumentForm.tsx Outdated Show resolved Hide resolved
src/components/item/form/DocumentForm.tsx Outdated Show resolved Hide resolved
@ztlee042 ztlee042 requested a review from spaenleh April 3, 2024 09:48
@spaenleh spaenleh requested a review from pyphilia April 3, 2024 14:27
spaenleh
spaenleh approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@spaenleh spaenleh left a comment

Choose a reason for hiding this comment

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

Changes look good as far as I am concerned. I think we need to merge the fonts and typography variants before this PR.

pyphilia
pyphilia approved these changes Apr 4, 2024
View reviewed changes
Copy link
Contributor

@pyphilia pyphilia left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I've left some suggestions, let me know if you have questions.

src/langs/en.json Outdated Show resolved Hide resolved
src/langs/en.json Outdated Show resolved Hide resolved
src/components/main/NewItemModal.tsx Outdated Show resolved Hide resolved
@ztlee042
Copy link
Contributor Author

ztlee042 commented Apr 4, 2024 •
edited
Loading

CleanShot 2024-04-04 at 14 20 25

Fix Select

  • Add icons
  • Apply colors via theme
  • Change selected item style to bordered
  • Select 'standard' by default

Future work

  • Change the hover color on the selected item, the current one is a purple-ish color, which is inconsistent with the grey-ish color on unselected items
  • Align the select, menu items with the text area

@ztlee042
Copy link
Contributor Author

ztlee042 commented Apr 4, 2024

Add Tooltip

  • move helper text to tooltip
  • remove the built-in required feature since it takes another row instead on the same row with the new label
  • add a custom required start
    CleanShot 2024-04-04 at 14 53 45

@ztlee042 ztlee042 requested review from spaenleh and pyphilia April 4, 2024 12:55
pyphilia
pyphilia approved these changes Apr 4, 2024
View reviewed changes
Copy link
Contributor

@pyphilia pyphilia left a comment

Choose a reason for hiding this comment

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

All good to me, you just need to rebase and we can merge this branch.

src/langs/en.json Show resolved Hide resolved
@ztlee042
Copy link
Contributor Author

ztlee042 commented Apr 4, 2024

All good to me, you just need to rebase and we can merge this branch.

I rebased, hope I get it right :)

@ztlee042 ztlee042 requested a review from pyphilia April 4, 2024 16:08
ztlee042 and others added 17 commits April 5, 2024 12:56
@spaenleh spaenleh force-pushed the 1096-redesign-add-text branch from 4ff109e to 7453232 Compare April 5, 2024 11:01
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Apr 5, 2024

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@pyphilia pyphilia added this pull request to the merge queue Apr 5, 2024
Merged via the queue into main with commit 514fbaf Apr 5, 2024
4 checks passed
@github-actions github-actions bot mentioned this pull request Apr 5, 2024
Merged
@pyphilia pyphilia deleted the 1096-redesign-add-text branch April 5, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pyphilia pyphilia pyphilia approved these changes

@spaenleh spaenleh Awaiting requested review from spaenleh

Assignees

@ztlee042 ztlee042

Labels
v2.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ztlee042 @pyphilia @spaenleh