Update how About License information is displayed in EditSourceModal #4716

LianaHarris360 · 2024-09-06T20:25:27Z

Summary

Description of the change(s) you made

This PR resolves the confusion surrounding the question mark icon in the disabled License input field, which previously made it unclear if the question mark icon was interactive. It ensures that even when the input is disabled, users can still access information about licenses and removes the modal opening another modal functionality.

Screenshots (if applicable)

Before:

AboutLicensesBefore.mov

After:

AboutLicensesAfter.mov

References

Closes #4679

Comments

Removed AboutLicensesModal.vue and contents within to LicenseDropdown.vue
Removed mutation SET_SHOW_ABOUT_LICENSES and references.
Removed isAboutLicensesModalOpen getter.
Added basic-link KButton to LicenseDropdown.vue and toggling functionality.

Contributor's Checklist

PR process:

If this is an important user-facing change, PR or related issue the CHANGELOG label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later time
If this includes an internal dependency change, a link to the diff is provided
The docs label has been added if this introduces a change that needs to be updated in the user docs?
If any Python requirements have changed, the updated requirements.txt files also included in this PR
Opportunities for using Google Analytics here are noted
Migrations are safe for a large db

Studio-specifc:

All user-facing strings are translated properly
The notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)
All UI components are LTR and RTL compliant
Views are organized into pages, components, and layouts directories as described in the docs
Users' storage used is recalculated properly on any changes to main tree files
If there new ways this uses user data that needs to be factored into our Privacy Policy, it has been noted.

Testing:

Code is clean and well-commented
Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Any new interactions have been added to the QA Sheet
Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

marcellamaki

Hi Liana - this looks good thank you for working on it. I also did manual QA and it is working as expected, and since these strings were pre-existing, I did the review in Spanish and confirmed that they were all being imported and referenced correctly.

A (very tiny) nitpick change and also a clarifying question, but overall this seems more-or-less ready to go.

marcellamaki · 2024-09-09T13:30:18Z

contentcuration/contentcuration/frontend/shared/views/LicenseDropdown.vue

    },
+    data() {
+      return {
+        reqeuestFormStrings: crossComponentTranslator(RequestForm),


A small nitpick and a curiosity question:

there's a typo here reqeuest (it's working, though! the spelling is consistent. but a nice small cleanup)

It seems like even though I had problems with the modal interaction previously, it was working for you with the correct text (modal-in-modal, as originally designed). I'm wondering how the strings were working? I don't see crossComponent translator used in the deleted file. It's not a problem! I'm just trying to figure it out for the sake of a comprehensive review 😄

Thanks for catching this, not sure how I missed it!

I believe the problems you were having was because the License input was disabled. I noticed this happening when it is unable to be edited I think because it is not in its original location. The strings worked fine but they were directly in the deleted file so instead of adding them over again in LicenseDropdown.vue, I decided to use the crossComponent translator. I was unsure if adding the same strings to LicenseDropdown.vue would import the translated strings the correct way.

okay thank you @LianaHarris360!

AlexVelezLl

Code changes looks good to me! Just noted a little thing with the styles of the dropdown icon.

I think its also worth noting that this change is also going to change the edit all details modal, I guess this is the intended behaviour, but just noting it just in case 😅.

AlexVelezLl · 2024-09-09T15:28:01Z

contentcuration/contentcuration/frontend/shared/views/LicenseDropdown.vue

          :hide-selected="isMixedLicense"
          :menu-props="{ ...menuProps, maxHeight: 250 }"
          class="ma-0"
          :class="{ 'with-trailing-input-icon': box }"


We had this class to include the trailing icon for the about licenses modal. As we are removing this, we need to remove also this line and/or fix these alignment styles, and all the related styles here https://github.com/LianaHarris360/studio/blob/0cefdcec13a859cb739914fdb6cd0fabb65c99d9/contentcuration/contentcuration/frontend/shared/views/LicenseDropdown.vue#L242, as this cause this little misalign in the dropdown icon.

Thanks for pointing this out, I was wondering about this misalignment but didn't initially see where it was being set. Now I can get it fixed!

The input selection arrow alignment is now fixed :)

Hi Liana! It seems we now have this little bug when we select an option with less width 😅 :

I believe this bug was present before these changes were made. I attempted to edit the source on unstable and encountered the same issue. Adding a minimum width fixes it! I also noticed it had a slightly darker background than the rest of the inputs in the EditSource modal and updated that as well.

EditSourceLicense.mov

Full Edit Details page view:

marcellamaki · 2024-09-10T13:49:11Z

this change is also going to change the edit all details modal, I guess this is the intended behaviour, but just noting it just in case - @AlexVelezLl

hmmmmm. this is a good point, @AlexVelezLl, thank you for raising. I don't know if we want to change this within the edit modal or not. @LianaHarris360 can I think on this for a day? I don't think it's necessarily a problem if we do. I just want to make sure we're being intentional. (cc @akolson and @radinamatic)

akolson · 2024-09-12T10:50:04Z

I think this is ready to merge. Any additional issues, which I don’t see as blockers, can be addressed in a follow-up issue. cc @marcellamaki.

@AlexVelezLl could you please create an issue with the details and approve this so it can be merged?

Thanks @LianaHarris360 for the fix and follow through.

AlexVelezLl

Thanks Liana! Looks good to me! @akolson I son see any aditional issues, just finishing the decisioin whether we want this version in the "edit all details" modal, although imo there is no problem with that, and it would be good for concistency. Thanks!

LianaHarris360 added 2 commits September 6, 2024 15:12

Move content of AboutLicenseModal to LicenseDropdown with toggle button

a85aac1

Remove AboutLicenseModal and references

0cefdce

LianaHarris360 added this to the Studio: Bulk Editing Release milestone Sep 6, 2024

LianaHarris360 requested review from AlexVelezLl, bjester and marcellamaki September 6, 2024 20:25

marcellamaki reviewed Sep 9, 2024

View reviewed changes

AlexVelezLl reviewed Sep 9, 2024

View reviewed changes

Fix string typos and license input selection misalignment

a3a0fe0

LianaHarris360 requested review from AlexVelezLl and marcellamaki September 9, 2024 18:10

marcellamaki self-assigned this Sep 10, 2024

Add mininmum width to license dropdown and adjust background color

78221d4

AlexVelezLl approved these changes Sep 12, 2024

View reviewed changes

LianaHarris360 merged commit 1bb1197 into learningequality:unstable Sep 12, 2024

This was referenced Sep 12, 2024

Revert "Update how About License information is displayed in EditSourceModal" #4730

Merged

About Licenses display changes + revise license URL return method to improve security #4731

Merged

Update how About License information is displayed in EditSourceModal #4716

Update how About License information is displayed in EditSourceModal #4716

Uh oh!

Conversation

LianaHarris360 commented Sep 6, 2024

Summary

Description of the change(s) you made

Screenshots (if applicable)

References

Comments

Contributor's Checklist

Reviewer's Checklist

This section is for reviewers to fill out.

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

marcellamaki Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

marcellamaki Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcellamaki commented Sep 10, 2024

Uh oh!

akolson commented Sep 12, 2024

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AlexVelezLl left a comment •

edited

Loading

LianaHarris360 Sep 10, 2024 •

edited

Loading