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.

Sign up for GitHub

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

Jump to bottom

GO-200/admin_others_target_UX #52

Merged
merged 24 commits into from
Sep 22, 2023
Merged

GO-200/admin_others_target_UX #52

merged 24 commits into from
Sep 22, 2023

Conversation

stage-rl
Copy link
Collaborator

Key changes

  • all admin entities refactored to target UX
  • structural changes to boxes - added color, short_name
  • structural changes to tags - added external flag
  • improved login screen to turbo based modal - works for all screens

@stage-rl stage-rl requested a review from jsuchal September 18, 2023 16:40
jsuchal
jsuchal requested changes Sep 20, 2023
View reviewed changes
Copy link
Member

@jsuchal jsuchal left a comment

Choose a reason for hiding this comment

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

Dost naprd sa to cita, ked su tam este prilepene nejake zmeny od mirreca a lucie, ale nevidim tam vela pruserov. Mas komenty.

app/components/admin/permissions/group_form_component.html.erb Outdated Show resolved Hide resolved
app/components/admin/permissions/group_form_component.html.erb Show resolved Hide resolved
app/components/admin/permissions/group_form_component.html.erb Outdated
</div>
<%= tag.turbo_frame id:'tag-search-results' %>
<%= form.search_field :name_search, value: params[:name_search], placeholder: "Vyhľadaj štítok",
oninput: "this.form.requestSubmit()",
Copy link
Member

Choose a reason for hiding this comment

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

Tu si dajme poznamku, ze pri autocomplete sa zvykne robit aj nejaky debounce.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Autocomplete som pridal. Vid solver-it-sro@cb2627d

app/components/admin/permissions/groups_list_component.html.erb Outdated
</div>
<div class="self-stretch flex-col justify-start items-start flex">
<% @groups.each do |group| %>
<%= render Admin::Permissions::GroupsListRowComponent.new(group) %>
Copy link
Member

Choose a reason for hiding this comment

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

Tu by moholo zafungovat https://viewcomponent.org/guide/collections.html#collections

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prerobil som tie, co si oznacil. Ale je to na vacsi refactor niekedy. Su ich tam desiatky

app/components/admin/permissions/groups_list_row_component.html.erb Show resolved Hide resolved
app/controllers/messages_controller.rb Outdated
@@ -6,6 +6,7 @@ def show

@message.update(read: true)
@message_thread = @message.thread
@available_tags = available_tags
Copy link
Member

Choose a reason for hiding this comment

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

nemalo by toto byt lazy cez turbo? ze sa to nebude tahat vzdy, ked to vlastne treba len ak sa ide editovat tagy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, mas cez Turbo

app/helpers/box_tailwind_color_helper.rb Outdated
@@ -0,0 +1,27 @@
module BoxTailwindColorHelper
Copy link
Member

Choose a reason for hiding this comment

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

Zvlastny nazov. V klude by som ich tu vypisal do pola (to by mal tailwind chytat) a dal tu helper ktoremu hodis nejaky name a on vygeneruje class podla hash. pripadne to presunut do Box rovno kde sa to generuje (V tom post create hook)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Uz som ho vymazal. Len je to v tej dalsej branchi k vyberu boxu. Dal som to do komponentu, ktory zobrazuje ten farebny label tagu.

app/policies/admin/tag_group_policy.rb Outdated Show resolved Hide resolved
app/views/boxes/show.html.erb Outdated Show resolved Hide resolved
app/views/layouts/application.html.erb Outdated
@@ -22,5 +22,6 @@
<% end %>
<% end %>
<%= turbo_frame_tag :modal %>
<%= turbo_frame_tag :login_screen %>
Copy link
Member

Choose a reason for hiding this comment

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

login screen ma byt imho vlastny layout

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, tak som mu spravil, ale stale teda cakame login screen od UXakov

@stage-rl stage-rl requested a review from jsuchal September 22, 2023 05:57
@jsuchal
Copy link
Member

jsuchal commented Sep 22, 2023

@stage-rl nevies tu zmenit target na tu tvoju branchu z ktorej to vychadza alebo aspon mergnut ten velky PR predtym? 216 suborov znova citat fakt nejdem.

@stage-rl stage-rl changed the base branch from GO-183/admin_users_target_UX to main September 22, 2023 06:45
jsuchal
jsuchal reviewed Sep 22, 2023
View reviewed changes
app/jobs/drafts/parse_import_job.rb Outdated
@@ -77,9 +77,11 @@ def load_import_csv(import, csv_path)
last_message_delivered_at: Time.now
)

# TODO: Nemali by sme tento vytvorit hned s tenantom? Co ked niekto zalozi rucne medzitym? A to external je v tomto pripade matuce, mozno by to malo byt "system". Povedzme si
Copy link
Member

Choose a reason for hiding this comment

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

Ano, za mna systemove by mali vzniknut hned ako vznikne tenant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, hotovo.

jsuchal
jsuchal reviewed Sep 22, 2023
View reviewed changes
db/migrate/20230912140629_add_color_to_boxes.rb Outdated
t.enum :color, enum_type: 'color'
end
Box.all.each do |box|
box.color = Box.colors.keys[Digest::MD5.hexdigest(@box.name).to_i(16) % Box.colors.size]
Copy link
Member

Choose a reason for hiding this comment

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

Tu je preco stale ten md5?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To je jednorazovka na starych boxoch, nevadilo by. Ale hotovo.

@stage-rl stage-rl requested a review from jsuchal September 22, 2023 09:10
@stage-rl stage-rl merged commit 4e416a5 into main Sep 22, 2023
2 checks passed
@jsuchal jsuchal deleted the GO-200/admin_others_target_UX branch September 22, 2023 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsuchal jsuchal jsuchal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stage-rl @jsuchal