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

wip #18

Closed
wants to merge 6 commits into from
Closed

wip #18

wants to merge 6 commits into from

Conversation

evykassirer
Copy link
Owner

Fixes:

Screenshots and screen captures:

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@evykassirer evykassirer force-pushed the settings-users-typescript branch 4 times, most recently from c4b4b4a to d77e685 Compare November 5, 2024 03:17
@evykassirer
util: Move compare_a_b from user_sort and generalize it.
e84fe70
@evykassirer
settings_users: Make separate variables for each section.
9a7c948 
This will help with the conversion to typescript.
@evykassirer
settings_users: Initialize full section objects.
21cd3a6 
Typescript likes objects to be fully initialized up front,
instead of adding functions to them later.
@evykassirer evykassirer force-pushed the settings-users-typescript branch from d77e685 to eef9318 Compare November 5, 2024 03:50
@evykassirer
settings_users: Don't use text() with undefined value.
4216ef0 
`text(undefined)` is a noop, and also isn't validly typed.
@evykassirer evykassirer force-pushed the settings-users-typescript branch from eef9318 to cc79590 Compare November 5, 2024 04:00
@evykassirer
settings_users: Remove unnecessary bot user undefined check.
2729de0 
Users are fetched from `people_by_user_id_dict`.

`bot_info` is only called from:

* `bots_create_table`, with ids from `get_bot_ids` which are
  from this `people_by_user_id_dict`.
* `update_bot_data`, which is called in two places that check
  `is_valid_bot_user` on the bot id first.

So we should always get a `bot_user` back, and we're already relying
on the bot users being defined in several methods of the
`bot_list_widget`.
@evykassirer
settings_users: Convert module to typescript.
d7aa11f
@evykassirer evykassirer force-pushed the settings-users-typescript branch from cc79590 to d7aa11f Compare November 5, 2024 06:24
@evykassirer evykassirer closed this Nov 5, 2024
@evykassirer evykassirer deleted the settings-users-typescript branch November 7, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@evykassirer