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

Fixes #7491 - moved default org and default location to same tab as org/... #1774

Closed
wants to merge 1 commit into from

Conversation

shlomizadok
Copy link
Member

...loc selection

@shlomizadok
Copy link
Member Author

[test]

@isratrade
Copy link
Member

@shlomizadok, looks good, but I noticed two issues.

  1. when you de-select a location in the multi-select , the default location changes from a drop-down to a multi-select widget
  2. for admin users, default location shows all locations (which it should) but then after selecting a location in the multi-select, the default location filters to only selected locations as would be for non-admins. This behaviour also appears in the 'develop' brunch so it may be a separate issue/PR.

@@ -9,6 +9,11 @@
{:disabled => obj.used_location_ids }.merge!(select_options),
{'data-descendants' => obj.children_of_selected_location_ids.to_json,
'data-useds' => obj.used_location_ids.to_json }.merge!(html_options[:location]) %>

<% if SETTINGS[:locations_enabled] && (obj.kind_of? User) %>
<%= select_f f, :default_location_id, (obj.admin? ? Location.all : obj.locations), :id, :title,
Copy link
Member

Choose a reason for hiding this comment

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

for non admins, I am guessing this should only show the selected taxonomies, rather then the one in the db?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does that on the client side. When loaded it show user's taxonomy and changes on the client when selecting / unselecting a taxonomy.

:html_options => { :location => {:onchange => 'taxonomy_added(this, "location")' },
:organization => {:onchange => 'taxonomy_added(this, "organization")' }
} %>
:html_options => user_taxomonies_html_options(@user)
Copy link
Member

Choose a reason for hiding this comment

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

s/taxonomonies/taxonomies/g , same in the Javascript file.

@dLobatog
Copy link
Member

@isratrade I believe the reason why it goes from multi-select to dropdown is because the number of taxonomies you have selected, and therefore available to select as default, is less than 5.

@shlomizadok I'm 👌 with merging this code after fixing the typo. It might make more sense to label "default organization" as "default on login", since it's already in the organization tab it's clear it's a location. Opinions? @domcleal (you reviewed the intial version of this) maybe?

@shlomizadok
Copy link
Member Author

@elobato - fixed the typo. Thanks.

@@ -9,6 +9,10 @@
{:disabled => obj.used_location_ids }.merge!(select_options),
{'data-descendants' => obj.children_of_selected_location_ids.to_json,
'data-useds' => obj.used_location_ids.to_json }.merge!(html_options[:location]) %>
<% if SETTINGS[:locations_enabled] && (obj.kind_of? User) %>
<%= select_f f, :default_location_id, (obj.admin? ? Location.all : obj.locations), :id, :title,
{ :include_blank => true }, { :label => _('Default location') } %>
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this label (and the one for organizations) to "Default on login"?
Since you moved this to the tabs they belong in (thanks!) "Default location" is redundant with the title of the tab, and "Default on login" makes it a bit clearer for users to understand what's this default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the labels.

@dLobatog
Copy link
Member

Merged as 79f0876 , thanks @shlomizadok !

@dLobatog dLobatog closed this Sep 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants