- Notifications
You must be signed in to change notification settings - Fork413
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Improve UI for sorting options on comments#13670
Improve UI for sorting options on comments#13670
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This pull request does not contain a valid label. Please add one of the following labels:['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
Hi@greenwoodt! |
If you have a solution, then yes this method would be fantastic! Thanks@andra-panaite! |
…ting-dropdown-for-comments
@andra-panaite After further internal discussion regarding changes to blend the new Sort By string, removing thebold-font from the selection drop down will be enough to approve the PR to merge! |
@greenwoodt I have removed the bold font from the select tag and now it looks like this: Is this the desired result? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍🏽
The current behavior is weird, as it isn't the same when going the first time, and when changing it + refreshing it. It's a consistency issue problem at least. sort-by.webm@carolromero can you check this out and confirm that this is the intended behavior? |
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
…ting-dropdown-for-comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I have some suggestions, can you check them out?
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
decidim-comments/app/cells/decidim/comments/comments/order_control.erb OutdatedShow resolvedHide resolved
…ting-dropdown-for-comments
…emend-cofe/decidim into feature/sorting-dropdown-for-comments
* Added "Sort by" into the select dropdown* Resolved comment* Run linter* Removed bold style from select* Resolved comment* Added changes only to mobile view* Running linters* Removed duplicated id* Fixed accessibility issues* Apply suggestions from code review* Apply review recommendation* Running linters* Fix accessibility---------Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
* develop: (27 commits) WCAG navigation submenu (#13796) Update gem dependencies (part 3) (#13849) Prevent notifications for deleted users (#13812) Retries NPM installation a couple times to prevent network timeouts (#13831) Update gem dependencies (part 2) (#13839) Enhance signature pdf export (#13778) Fix HexaPDF dependency (#13834) Fix flaky spec in authentication (#13827) Merge upload field for documents and image on proposal admin form (#13735) Update gem dependencies (#13835) Upgrade erb_lint to 0.8.0 (#13833) Fix flaky spec in geocoder (#13820) Refactor modules mounting routes (#13294) Upgrade check-spelling action (#13825) Add missing images in the custom registration emails from meetings (#13632) Add missing translations (#13793) Fix proposal map performance with hundreds of markers (#13798) Create multiple surveys within same Survey component (#13420) Accountability bulk actions (#13730) Improve UI for sorting options on comments (#13670) ...
🎩 What? Why?
I have added a new option, "Sort by," that is disabled within the select. I have also added styling to bold the default option for sorting that is used when the user does not choose from the dropdown.
📌 Related Issues
Link your PR to an issue
Testing
📷 Screenshots
Please add screenshots of the changes you are proposing
