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

Fix incorrectly colored save button #2393

Merged
merged 3 commits into from
Dec 7, 2024
Merged

Conversation

sephirothkod
Copy link
Collaborator

Simpleform injects css classes into its button tags and those classes conflict with what is used in Hyku. The forms on the Pages page were using the button tag instead of the submit tag and causing the classes to be duplicated and conflict with each other. By using the submit tag it doesn't inject classes and only applies the classes specified in the tag.

Button color for "Save changes" button was white instead of blue like the rest of Hyku. This fixes that issue as well as issues in the future stemming from this "class injection".

@samvera/hyku-code-reviewers

…jecting classes into the button. Fixes button color being white instead fo blue.
@sephirothkod sephirothkod added the patch-ver for release notes label Nov 28, 2024
Copy link

github-actions bot commented Nov 28, 2024

Test Results

    3 files  ±0      3 suites  ±0   17m 36s ⏱️ -21s
2 053 tests ±0  1 997 ✅  - 6  56 💤 +6  0 ❌ ±0 
2 080 runs  ±0  2 022 ✅  - 6  58 💤 +6  0 ❌ ±0 

Results for commit eb21c2e. ± Comparison against base commit f76a5d5.

This pull request removes 45 and adds 45 tests. Note that renamed tests count towards both.
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 77435634-9f8a-4df5-b52a-640a706aab69
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit cc718ba4-1958-499c-b948-1313ebb5dab9
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 88a4c3a2-26da-4610-b74d-c351fd8ee64b
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update c053bd4f-991b-4ec9-981c-8fd1076e3f4a
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy bf1206b2-4a11-4851-9251-525c94f6a5e7
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit 2150fe91-65be-4f84-be1b-d672261d506b
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 6bca7913-538c-4f66-973b-f9c0a62e4da2
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 3aaff1b3-1060-4ece-b98e-0c50428811cf
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy 8ff865c6-c83d-42ff-a9f2-a189c221aac3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit e90a89e4-252b-4ddf-b5f2-f1efaee9c3f8
…
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 002c48d6-8002-4eb8-8e48-44482cd6d8cf
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit ea819bf3-3d13-4c75-915b-d160d0d1f767
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read a626019c-548c-4655-9f9c-9bd7aff7ee46
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update e3d2fe04-e937-4fe3-adec-289965b8d4a0
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 1a20f1ec-0407-4d3a-ac61-475899e6da30
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit 1f73322c-49a9-41ff-ac2c-f76df1e11a39
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 48b1b2b4-e904-4038-b453-0f1228329204
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 51f73e96-80a4-4d7a-a03e-97203169157e
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy fbb176c0-2ab0-4dd6-bf67-ac7788fe594a
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit b63c7502-f535-41ea-b767-a4f1fbc6019e
…
This pull request skips 3 tests.
spec.features.appearance_theme_spec ‑ Admin can select home page theme when a search results theme is selected updates to the users preferred view
spec.features.create_etd_spec ‑ Create a Etd a logged in user with the :work_depositor role can create an Etd
spec.features.work_editor_role_spec ‑ Work Editor role create permissions can create a work

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@bkiahstroud bkiahstroud 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 the PR!

The way you've described the issue makes me wonder if this is happening elsewhere in Hyku as well... if the fundamental issue is a conflict between Hyku theming and simpleform, I think changing the class names Hyku uses would make more sense. Would something like prefixing the classes Hyku uses with hyku_ be enough to prevent the conflict?

@sephirothkod
Copy link
Collaborator Author

Thanks for the PR!

The way you've described the issue makes me wonder if this is happening elsewhere in Hyku as well... if the fundamental issue is a conflict between Hyku theming and simpleform, I think changing the class names Hyku uses would make more sense. Would something like prefixing the classes Hyku uses with hyku_ be enough to prevent the conflict?

The classes that I am referring to are the bootstrap classes so it is part of a library that we can't really change.

@sephirothkod
Copy link
Collaborator Author

@bkiahstroud Just wanted to add that I tried new classes and the result is the same, it seems that it has something to do with the order in which the classes are, that it gives priority to the injected classes. The other option is to disable it in the config but then it removes it from all the buttons that use it which would cause us to have to do a lot of work when simply using f.submit instead of f.button works just as well. The devs comment on that here with another option to build a new button class for simple form. Which we could do in the future if we wanted.

Copy link
Collaborator

@bkiahstroud bkiahstroud left a comment

Choose a reason for hiding this comment

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

That makes sense to me. In that case, would you just mind adding some OVERRIDE comments around the lines that changed? We often grep for OVERRIDE when doing Hyrax upgrades

app/views/hyrax/pages/_form.html.erb Outdated Show resolved Hide resolved
@sephirothkod sephirothkod merged commit a1d7cef into main Dec 7, 2024
8 checks passed
@sephirothkod sephirothkod deleted the 2353-pages_save_button_css branch December 7, 2024 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants