Skip to content

Conversation

@tecoholic
Copy link
Owner

@tecoholic tecoholic commented Nov 30, 2024

Duplication of #125 after the diff got messed up for some reason.

This is based on #122 adding a bit more clean up and polish to the UI.

Before

Screenshot 2024-11-30 at 14-18-36 NER Annotator for SpaCy

After

Screenshot 2024-12-01 at 15-31-30 NER Annotator for SpaCy

@tecoholic tecoholic self-assigned this Nov 30, 2024
@tecoholic tecoholic requested a review from alvi-khan November 30, 2024 03:23
@tecoholic
Copy link
Owner Author

@alvi-khan Hi, when you get some free time, can you kindly review this one?

P.S: Desktop artifacts can be downloaded from the "Checks" > "Summary"

@alvi-khan
Copy link
Collaborator

Hey @tecoholic. The UI updates look great! I added a few minor opinions I had above.

Two more things. Firstly, I guess the link under the Screenshots section in the README.md file needs to be updated to ![NER Annotator Screenshot](./public/assets/step-2.png). GitHub wasn't letting me add this as a comment since that line wasn't modified.

Secondly, I think the windows build is failing because of the version number. It seems this is a known issue. I guess we'll have to manually upload the windows builds to the releases section until this version is ready for a full release.

@tecoholic
Copy link
Owner Author

@alvi-khan Thank you for a detailed review. I fixed a number of issues you pointed out and also made a few adjustments to the start page, including disabling the menu items on start page - only "Open File" is allowed.

  • Yes. The screenshot link is broken in the README. I think we need to update the screenshots for HelpDialog as well. So, I am planning on doing that once we have the functional changes reviewed and finalized.
  • Windows builds - Yeah. That's annoying. I don't have a Windows machine for doing Windows build. Do you happen to have one? And would you be able to test it out? Otherwise, I can remove the pre-release tag from the version and let GitHub handle it.
  • I am also thinking of adding a MacOS build for the 2.0 release. What do think about it? There is no work from our side to do it, Tauri's build should handle that in the CI.

@alvi-khan
Copy link
Collaborator

@tecoholic

  • Regarding the screenshot link in the README, looks like our community beat you to it!
  • For Windows builds, I do have a Windows machine available. Once the release is published I can edit it and add the Windows installers no worries.
  • I think adding a MacOS build would be great! And since the build itself shouldn't take extra work I see no reason to not add it. I do have some concerns regarding Mac-specific issues that might pop up since I don't have a Mac device available to test on which might make debugging a pain.

@tecoholic
Copy link
Owner Author

tecoholic commented Dec 2, 2024

@alvi-khan I generally agree with your comments on colors. Would you be kind enough to update the PR with the colors you think would be appropriate? I am generally color challenged and that's the reason, I tend to stick to defaults for most things. I tried to communicate certain things with color, but seems to have failed terribly 🫨

I have access to Macs both Intel and ARM ones. So, I can test the builds. No worries.

@alvi-khan
Copy link
Collaborator

@tecoholic Hey, I struggle with colors too — it's tricky! And you're being way too hard on yourself. The UI you built, both before and in this update, looks amazing! The modern and user-friendly design you gave this app is a big reason I decided to use it in the first place.

I’ve made a few minor adjustments to the colors where I thought it might help. If you’re good with these changes, I think this is ready to be merged. Thanks so much for putting in all this effort — it really shows!

@tecoholic tecoholic merged commit f546965 into main Dec 3, 2024
3 checks passed
@tecoholic
Copy link
Owner Author

@alvi-khan Thanks for your kind words and taking the time to make the fixes. You have been a great support in maintaining this project.

Version 2.0.0 released :)

@tecoholic tecoholic deleted the ui-refresh branch December 3, 2024 03:37
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.

3 participants