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

SF-2986 Add step to confirm language codes before generating draft #2730

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Nateowami
Copy link
Collaborator

@Nateowami Nateowami commented Sep 21, 2024

@josephmyers There are two designs here. One is based on a grid with three quadrants filled, and one is based on two rows, with the first row having a right and left. I like the concept @josephmyers came up with, but I also feel like the layout it produces is harder to understand. The structure of the page doesn't seem as clear to me. It's like there's just a bunch of text floating around and it's hard to tell what is associated with what. Feel free to fork this and propose changes.

Below screenshots are outdated

Implementation of Balsamiq mockup

Other concept that I worked on (on this branch, one commit behind the current tip of this branch)


This change is Reviewable

@Nateowami Nateowami force-pushed the feature/SF-2986-confirm-language-codes branch from 0c5df8c to cd879a6 Compare October 8, 2024 01:58
@Nateowami Nateowami marked this pull request as ready for review October 8, 2024 01:59
@Nateowami Nateowami force-pushed the feature/SF-2986-confirm-language-codes branch from cd879a6 to 07e0404 Compare October 8, 2024 02:07
@josephmyers josephmyers self-assigned this Oct 8, 2024
Copy link
Collaborator

@josephmyers josephmyers 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 poking me on this. I've made a new branch off this one with my layout touchups. I think the core here is excellent, but I tweaked the sizing, colors, and other styles for various items to better visually organize it. They are all aesthetic tweaks, I think, except that I did remove the descriptions. I found them to clutter up the page, but I understand that there may in fact be a need for them, though they're not in the accepted mockup. If you do want to maintain descriptions, we should limit them to two, one just above each gray box.

Finally, about the wording, I was wondering if we'll run into localization problems with "Drafting from," as you mentioned for my "Scripture Forge will be trained that..." If we could improve this wording at all, that'd be great, though I don't want to hold things up too long. I think that the use of "language model" in the first step is isolated and a little obscure, but we may be able to mitigate that by reusing the term here?

Reviewable status: 0 of 8 files reviewed, all discussions resolved

@Nateowami Nateowami force-pushed the feature/SF-2986-confirm-language-codes branch from 07e0404 to 4ea2821 Compare October 8, 2024 17:05
@Nateowami Nateowami force-pushed the feature/SF-2986-confirm-language-codes branch from 4ea2821 to 71f5d4b Compare October 16, 2024 22:23
@Nateowami
Copy link
Collaborator Author

@josephmyers I've incorporated your changes and cleaned things up, as well as adding some new ideas. Here's what it looks like now, in French:

Copy link
Collaborator

@josephmyers josephmyers left a comment

Choose a reason for hiding this comment

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

Yeah, this looks solid.

Reviewed 1 of 7 files at r2, all commit messages.
Reviewable status: 1 of 8 files reviewed, all discussions resolved

@Nateowami Nateowami force-pushed the feature/SF-2986-confirm-language-codes branch from 077a084 to 498fe72 Compare November 7, 2024 00:55
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