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

Aeh/upd circle #159

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Aeh/upd circle #159

wants to merge 3 commits into from

Conversation

aidan-hubley
Copy link
Owner

Changes

  • Restructured Circle Settings page
  • Added new functionality to page:
    • Edit the circle title / description (title is required, description isn't (use the same rules as the creation page))
    • Update the Circle icon, icon color, and circle color.

Notes

  • needs tested on iOS
  • Known bug: cannot functionally select border color when entering 'Edit Circle Icon' modal from circle, but when entering 'Edit Circle Icon' modal from button in 'Edit Circle' modal, the color selector works as intended.

YouTrack Link
PC-288 Edit Circle if Owner / Mod

@aidan-hubley aidan-hubley marked this pull request as ready for review April 10, 2024 04:03
@aidan-hubley aidan-hubley requested review from 02NRA and removed request for dev-arekusandoru April 19, 2024 04:42
@aidan-hubley
Copy link
Owner Author

@dev-arekusandoru can you check out the logic here? Both the 2 text inputs need a refactor (placeholders in the fields sometimes become the submitted values) and can you work on the circle color / icon updating

@aidan-hubley aidan-hubley requested review from dev-arekusandoru and removed request for 02NRA and dev-arekusandoru April 24, 2024 21:08
Copy link
Collaborator

@dev-arekusandoru dev-arekusandoru left a comment

Choose a reason for hiding this comment

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

this PR is not ready to go due to a few bugs:

  1. the text inputs are buggy and when you type it glitches out a lot. the placeholder and the default value should be equal to the title of the circe.
  2. if i change just the title or just the description it clears the other when saving. this shouldn't be happening.
  3. I don't think that the "circles dont need a description" notification is necessary, and it also is not displayed long enough to read.
  4. there is no way to get the keyboard to go away when editing the description
  5. I'm not sure what happens during the "Save" button press but it takes way to long for just a simple update of the database. if there are background processes happening we should still let the user use the app instead of staying in a frozen state
  6. I don't think its a good idea replacing the "Settings" page header with the circle name because 1) we lose the header of the page, 2) for longer titles we get a terrible overlap that also runs off the screen.
  7. i'm not sure what happened with the logic but now opening the settings page is visibly slow to load the data(check the beta, the data is already loaded and displayed when the page is navigated to)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants