Improvements to conversations with more than one participant by Leusev · Pull Request #6094 · decidim/decidim

Leusev · 2020-05-13T15:08:11Z

🎩 What? Why?

Acording to #5861 (Conversations with more than one person) issue, there's some improvements to be done.

This PR improves:

There are some aria roles missing from my previous example that could be useful in this case, like aria-owns (that links the input to the autocompleting options), aria-expanded (that tells the user when a control is expanded or not) or role="option" to indicate that somehing is an option in the expanded menu.
It looks like you can select the same user several times, shouldn't the selected ones be removed from the autocomplete list?

📌 Related Issues

Related to Conversations with more than one participant #5861

Notes

As in order to add aria accessibility funcionality for users dropdown auto-generated by Tribute.js lib, I had temporoliy modified tribute.js file as a inmediate fix while in the other hand I opened a PR to Tribute github repo with the same change:

Add some aria attribute to dropdown for accessibility purposes #469

📋 Subtasks

📷 Screenshots

tramuntanal · 2020-05-15T12:10:45Z

Hi @decidim/product this has been deployed into our staging

microstudi

Good job,
we just need to improve a little bit the GraphQL API additions. Please check the comments regarding the file user_entify_input_filter.rb.

A part from that comments, I'd like to modify the description in that file so it generates a more detailed documentation in /api/docs:

Proposed changes in user_entity_input_filter.rb:

Check http://localhost:3000/api/docs#UserEntityFilter to look for the generated result.

microstudi · 2020-05-21T09:27:35Z

decidim-core/app/types/decidim/core/user_entity_input_filter.rb

                   op_name.or(op_nick)
                 end
               end
+      argument :exclusion,


Suggested change

argument :exclusion,

argument :exclude_ids,

I'd change this for its imperative form, and being more specific about what's been used to exclude

microstudi · 2020-05-21T09:28:20Z

decidim-core/app/types/decidim/core/user_entity_input_filter.rb

               end
+      argument :exclusion,
+               type: String,
+               description: "Excludes users contained in given ids",


Suggested change

description: "Excludes users contained in given ids",

description: "Excludes users contained in given ids. Valid values are one or more IDs (passed as an arrays)",

This is used in the documentation, so let's try to be as clear as possible

microstudi · 2020-05-21T09:29:51Z

decidim-core/app/types/decidim/core/user_entity_input_filter.rb

                 end
               end
+      argument :exclusion,
+               type: String,


Suggested change

type: String,

type: [ID],

As we are using ID, we can specify array types of ID, this allows us to use a more idiomatic Graphql call:

users(filter:{excludeIds: [2 10 11]}) {

or simply:

users(filter:{excludeIds: 2}) {

microstudi · 2020-05-21T09:30:54Z

decidim-core/app/types/decidim/core/user_entity_input_filter.rb

+               required: false,
+               prepare: ->(value, _ctx) do
+                 proc do |model_class|
+                   value = value.split(",")


Suggested change

value = value.split(",")

We can remove this if using array type declaration

Leusev · 2020-05-22T08:43:14Z

It should be ok now @microstudi 😄

microstudi

There's only a correction on the camel case example calling the GraphQL

microstudi · 2020-05-22T10:47:38Z

decidim-core/app/types/decidim/core/user_entity_input_filter.rb

+
+      ```
+      {
+        users(filter:{wildcard:\"sandy\", exclude_ids:[2,10,11]}) {


Suggested change

users(filter:{wildcard:\"sandy\", exclude_ids:[2,10,11]}) {

users(filter:{wildcard:\"sandy\", excludeIds:[2,10,11]}) {

Leusev · 2020-05-25T06:59:07Z

It should be ok now @microstudi 😄

microstudi · 2020-05-25T15:30:36Z

Thanks!

* develop: Include year in meetings card (decidim#6102) Add attachment enabled option to initiative types and initiatives (decidim#6036) Fix a flaky test in group profile conversations (decidim#6123) Add attachments to Initiatives (decidim#5844) Add initiatives export (decidim#6070) Improvements to conversations with more than one participant (decidim#6094) Elections module and election administration (decidim#6065) Separate forms in steps (decidim#6108) Add sorting by publishing date to initiatives (decidim#6016) Improve proposal preview: Use proposal card when previewing a proposal draft (decidim#6064) Newsletter templates fixes (decidim#6096) # Conflicts: # decidim-initiatives/app/models/decidim/initiative.rb # decidim-initiatives/spec/system/admin/initiative_types_controller_spec.rb

* feature/add_areas_to_initiatives: (30 commits) Adds areas to FO filters Fix lint issue Fixes rubocop issues Updates changelog Adds areas to initiatives Send notification when signature threshold reached (decidim#6098) Adds filter by initiative type to admin panel (decidim#6093) Require confirmation on exiting a survey mid-answering (decidim#6118) Information message when there isn't any Proposal (decidim#6063) Set email asset host dynamically (decidim#5888) Harmonizes the design of initiatives search in FO (decidim#6090) Include year in meetings card (decidim#6102) Add attachment enabled option to initiative types and initiatives (decidim#6036) Fix a flaky test in group profile conversations (decidim#6123) Add attachments to Initiatives (decidim#5844) Add initiatives export (decidim#6070) Improvements to conversations with more than one participant (decidim#6094) Elections module and election administration (decidim#6065) Separate forms in steps (decidim#6108) Add sorting by publishing date to initiatives (decidim#6016) ... # Conflicts: # decidim-initiatives/app/cells/decidim/initiatives/initiative_m_cell.rb # decidim-initiatives/app/commands/decidim/initiatives/admin/update_initiative.rb # decidim-initiatives/app/controllers/decidim/initiatives/initiatives_controller.rb # decidim-initiatives/app/forms/decidim/initiatives/admin/initiative_form.rb # decidim-initiatives/app/helpers/decidim/initiatives/application_helper.rb # decidim-initiatives/app/models/decidim/initiative.rb # decidim-initiatives/app/services/decidim/initiatives/initiative_search.rb # decidim-initiatives/app/views/decidim/initiatives/create_initiative/fill_data.html.erb # decidim-initiatives/app/views/decidim/initiatives/initiatives/_filters.html.erb # decidim-initiatives/app/views/decidim/initiatives/initiatives/_tags.html.erb # decidim-initiatives/config/locales/en.yml # decidim-initiatives/db/migrate/20200514085422_add_area_to_initiatives.rb # decidim-initiatives/db/migrate/20200514102631_add_area_enabled_option_to_initiatives.rb # decidim-initiatives/spec/forms/initiative_form_spec.rb # decidim-initiatives/spec/services/decidim/initiatives/initiative_search_spec.rb # decidim-initiatives/spec/shared/update_initiative_type_example.rb # decidim-initiatives/spec/system/admin/admin_manages_initiatives_spec.rb # decidim-initiatives/spec/system/admin/initiative_types_controller_spec.rb # decidim-initiatives/spec/system/filter_initiatives_spec.rb

Add new API's User GraphQL filter to allow users exclusion

9e7eeda

Leusev linked an issue May 13, 2020 that may be closed by this pull request

Improvements to conversations with more than one participant #5994

Closed

Add CHANGELOG entry

efda5a5

Leusev added the status: WIP label May 13, 2020

Leusev added 6 commits May 14, 2020 10:53

Fix rubocop lint

35c87bf

Add multiple mentions users exclude list

0c01acc

Add rspec tests for GraphQL users query exclusion param

d7f7bfb

Modify tribute.js in order to add aria accessibility functions

e296a6b

Clean duplicate comment bars

d972155

Add aria accessibility attributes to add conversation's users dropdown

49449cd

Leusev marked this pull request as ready for review May 14, 2020 12:10

Leusev requested review from jesusdb and tramuntanal May 14, 2020 12:10

Leusev removed the status: WIP label May 14, 2020

Leusev added 2 commits May 14, 2020 14:12

Fix user entity input filter spec test

9046bcf

Fix js.es6 lint

33f789c

Leusev added 2 commits May 18, 2020 14:15

Fix user's exclusion dropdown bug

cd0b3d6

Add narrow column width for conversation detail

5011a6a

Leusev requested a review from microstudi May 19, 2020 13:23

Merge branch 'develop' into feat/conversations_improvements

3a91dd4

microstudi added the in-review label May 21, 2020

microstudi requested changes May 21, 2020

View reviewed changes

Leusev added 2 commits May 22, 2020 08:39

Merge with develop branch and conflict solve

ae1ab16

Improves GraphQL doc and exclude_ids param treatment

a9a7116

Leusev requested a review from microstudi May 22, 2020 09:41

microstudi requested changes May 22, 2020

View reviewed changes

Leusev added 2 commits May 25, 2020 08:57

Fix GraphQL doc issue

ef9cae2

Merge branch 'develop' into feat/conversations_improvements

bbe5953

microstudi previously approved these changes May 25, 2020

View reviewed changes

Merge branch 'develop' into feat/conversations_improvements

69c1b0a

microstudi dismissed their stale review via 69c1b0a May 25, 2020 15:35

microstudi previously approved these changes May 25, 2020

View reviewed changes

Merge branch 'develop' into feat/conversations_improvements

d4d7d27

microstudi dismissed their stale review via d4d7d27 May 25, 2020 16:50

microstudi approved these changes May 25, 2020

View reviewed changes

microstudi merged commit 95a4031 into develop May 25, 2020

microstudi deleted the feat/conversations_improvements branch May 25, 2020 16:50

microstudi removed the in-review label May 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improvements to conversations with more than one participant#6094

Improvements to conversations with more than one participant#6094
microstudi merged 19 commits intodevelopfrom
feat/conversations_improvements

Leusev commented May 13, 2020 •

edited

Loading

Uh oh!

tramuntanal commented May 15, 2020

Uh oh!

microstudi left a comment

Uh oh!

microstudi May 21, 2020

Uh oh!

microstudi May 21, 2020

Uh oh!

microstudi May 21, 2020

Uh oh!

microstudi May 21, 2020

Uh oh!

Leusev commented May 22, 2020

Uh oh!

microstudi left a comment

Uh oh!

microstudi May 22, 2020

Uh oh!

Leusev commented May 25, 2020

Uh oh!

microstudi commented May 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	description: "Excludes users contained in given ids",
	description: "Excludes users contained in given ids. Valid values are one or more IDs (passed as an arrays)",

	users(filter:{wildcard:\"sandy\", exclude_ids:[2,10,11]}) {
	users(filter:{wildcard:\"sandy\", excludeIds:[2,10,11]}) {

Uh oh!

Conversation

Leusev commented May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

Notes

📋 Subtasks

📷 Screenshots

Uh oh!

tramuntanal commented May 15, 2020

Uh oh!

microstudi left a comment

Choose a reason for hiding this comment

Uh oh!

microstudi May 21, 2020

Choose a reason for hiding this comment

Uh oh!

microstudi May 21, 2020

Choose a reason for hiding this comment

Uh oh!

microstudi May 21, 2020

Choose a reason for hiding this comment

Uh oh!

microstudi May 21, 2020

Choose a reason for hiding this comment

Uh oh!

Leusev commented May 22, 2020

Uh oh!

microstudi left a comment

Choose a reason for hiding this comment

Uh oh!

microstudi May 22, 2020

Choose a reason for hiding this comment

Uh oh!

Leusev commented May 25, 2020

Uh oh!

microstudi commented May 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Leusev commented May 13, 2020 •

edited

Loading