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

153: Attribute information improvements #193

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

ediazgallego
Copy link
Collaborator

@ediazgallego ediazgallego commented Sep 20, 2024

Why

Existing implementation: The existing form places the URL after the name requiring user to manually enter all fields

UX recommendations

  • Place URL as the first field

What:

  • Update AttributionInformation.tsx file to place the URL text input as the first field within the attribution form section.

Reference

How to test:

Screenshots

Before

Screenshot 2024-09-20 131412

After

Screenshot 2024-09-20 131503

Signed-off-by: ediazgallego <ediazgallego93@gmail.com>
Signed-off-by: ediazgallego <ediazgallego93@gmail.com>
@aevo98765
Copy link
Member

aevo98765 commented Sep 23, 2024

Are we going to implement the logic here to auto-populate the other attribution fields like suggested in the issue?

The re-ordering of the input fields looks good so great job with this!

@ediazgallego
Copy link
Collaborator Author

Are we going to implement the logic here to auto-populate the other attribution fields like suggested in the issue?

The re-ordering of the input fields looks good so great job with this!

Not very familiar how big or small the changes should be for each PR, but to me the auto-populate logic seems a different concern than fixing the formatting part, so maybe creating another issue that will focus on fixing that part could be a good idea, wdyt?

@aevo98765
Copy link
Member

Happy with this. I think the smaller increments we push to main the better.

Lets make sure that all the important contextual information get populated into the new issue. Put a link to the new issue in here when created. Then we will get this pushed through.

@ediazgallego
Copy link
Collaborator Author

Happy with this. I think the smaller increments we push to main the better.

Lets make sure that all the important contextual information get populated into the new issue. Put a link to the new issue in here when created. Then we will get this pushed through.

@aevo98765 here is the new issue created as a follow-up of the remaining work to complete the full logic of auto-populate behavior 195

@vishnoianil
Copy link
Member

@ediazgallego Tracking through different issue sounds good. Thanks for creating the issue.

This PR looks good to me.

Copy link
Member

@vishnoianil vishnoianil left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks for the contribution! @ediazgallego

@vishnoianil vishnoianil merged commit 5e4c212 into instructlab:main Sep 23, 2024
5 checks passed
@ediazgallego ediazgallego deleted the 153-attribute-improvements branch September 24, 2024 12:19
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.

Attribute information improvements
3 participants