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 new collective name input elements #551

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 10, 2023

📝 Summary

This resolves some styling issues, like hovering the submit input does not set the border on the text input or mismatching borders. Moreover the NcMultiselect is deprecated and should be replaced with the NcSelect.

🖼️ Screenshots

🏚️ Before 🏡 After
only submit button has error color, wrong border radius both elements input and submit have correct borders
hovering only highlights the submit button correct border color on whole element on hover
. submit button with correct borders
circle select separated from submit button submit button part of circle select
multselect no elements NcSelect no results

Please note that the missing primary color border on hover will be fixed with the next vue components update (see nextcloud-libraries/nextcloud-vue#3701 )

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@susnux susnux added design enhancement New feature or request 3. to review labels Feb 10, 2023
@susnux susnux changed the title Fix *new collective* input elements Fix new collective name input elements Feb 10, 2023
@cypress
Copy link

cypress bot commented Feb 10, 2023

Passing run #329 ↗︎

0 50 0 0 Flakiness 0

Details:

Fix `new collective` name input elements
Project: Collectives Commit: d27eed21d7
Status: Passed Duration: 04:14 💡
Started: Feb 15, 2023 6:16 PM Ended: Feb 15, 2023 6:21 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@mejo-
Copy link
Member

mejo- commented Feb 13, 2023

Wow, thanks a lot @susnux. I started work on completely replacing the new collective form with a modal though. Did you see that? #504

I guess that superseedes your PR, no?

It's still work in progress but if you fancy a review, that'd be highly appreciated nevertheless. I intend to finish and merge it this week (before next Collectives release).

@max-nextcloud
Copy link
Collaborator

I'd suggest to still merge this. It fixes ui bugs. If the ui is replaced even before the next release so be it. Maybe the modal takes longer than expected, maybe we want to use this input again later on. Even if not - i don't see any downside to merging this. But maybe I am missing something, @mejo-

Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

After some first testing, I identified the following issues.

src/components/Nav/NewCollective.vue Show resolved Hide resolved
src/components/Nav/NewCollective.vue Outdated Show resolved Hide resolved
src/components/Nav/NewCollective.vue Show resolved Hide resolved
@susnux susnux force-pushed the fix/new-collective-input branch 2 times, most recently from 0a29dd6 to d27eed2 Compare February 15, 2023 18:10
…d of custom input

This resolves some styling issues, like hovering the submit input does not set the border
on the text input or mismatching borders.
Moreover the `NcMultiselect` is deprecated and should be replaced with the `NcSelect`.

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@mejo-
Copy link
Member

mejo- commented Feb 20, 2023

Thanks a lot @susnux ❤️

@mejo- mejo- merged commit 8aabe9b into main Feb 20, 2023
@delete-merged-branch delete-merged-branch bot deleted the fix/new-collective-input branch February 20, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird borders on create collectives input element
3 participants