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

[Profile] My profile screens reorganisation and items design #8070

Closed
flexsurfer opened this issue Apr 29, 2019 · 42 comments · Fixed by #8825
Closed

[Profile] My profile screens reorganisation and items design #8070

flexsurfer opened this issue Apr 29, 2019 · 42 comments · Fixed by #8825
Labels
feature feature requests

Comments

@flexsurfer
Copy link
Member

flexsurfer commented Apr 29, 2019

Introduce items design and screens organisation
image

Privacy and security
image

Need help
image

About
image

Advanced
image

Contacts, Devices, Group chat, Notifications, Language screens, Advanced subscreens and crossed element are not in the scope of this PR

Acceptance criteria:

  1. pixel perfect
  2. screens reorganized and new design of items implemented

Figma:
https://www.figma.com/file/TNCyHKtR3sx5EL6YznFWUa4O/Profile?node-id=510%3A3116
https://www.figma.com/file/bPS9GrvMr7LnH7vnkmuveQfd/Settings?node-id=0%3A1

@flexsurfer flexsurfer mentioned this issue Apr 29, 2019
5 tasks
@flexsurfer flexsurfer added the feature feature requests label May 13, 2019
@jeluard
Copy link
Contributor

jeluard commented Jun 4, 2019

@flexsurfer Can you comment why some items are left out? Do we have any tasks to track them?

@rachelhamlin
Copy link
Contributor

rachelhamlin commented Jun 4, 2019

So as far as I can recall @jeluard -

  • Group chat invite controls are a new feature, so we're not including that here (only UI)
  • Same with Restore defaults
  • Opt-in web3 provider access no longer exists, but was still in the design at the time

Now designs need to be further updated to reflect the addition of Preview privacy mode and relocation of Browser privacy mode.

In other words, this screen should not have Group chat invites or Browser privacy mode and instead should have Preview privacy mode (which probably most users don't understand by name alone but that's another problem).

Add this setting instead:

image

@rachelhamlin
Copy link
Contributor

rachelhamlin commented Jun 4, 2019

I noticed that the new designs don't include DApp permissions anywhere. We should keep those, under Privacy and security > Privacy as well.

Currently they're in the top level menu:

So Privacy contains:

(1) Preview privacy mode
(2) DApp permissions

@flexsurfer
Copy link
Member Author

hey @jeluard so the idea was to split profile epic into atomic issues, we have issues to track other changes, but these issues should be reviewed before starting work, i believe these issues not in the priority for now anyway, so i'll review them again when we will start working on them

@StatusWrike
Copy link

➤ Julien Eluard commented:

Estimated to 2 days of work

@StatusSceptre
Copy link
Member

Hey @hesterbruikman @rachelhamlin @flexsurfer @jeluard

For bounties labeled as Large, I need some more info before I can assign a bounty

  • Proposed bounty amount = Large (16 hrs)
  • Estimated time to completion = Day
  • Estimated required experience level = ?
  • Why we’re outsourcing it = ?

Can you please add this info and I'll go ahead and get the bounty approved by Finance.

Thanks!

@jeluard
Copy link
Contributor

jeluard commented Jul 8, 2019

Hey @StatusSceptre ! Some more context:

  • Estimated time to completion: I estimated it to 2 days for someone familiar with the codebase
  • Estimated required experience level: Newbie is ok but obviously would require more investment
  • Why we’re outsourcing it: lack of resource and low risk?

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 800.0 DAI (800.0 USD @ $1.0/DAI) attached to it.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Workers have applied to start work.

These users each claimed they can complete the work by 12 months from now.
Please review their action plans below:

1) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Workers have applied to start work.

These users each claimed they can complete the work by 12 months from now.
Please review their action plans below:

1) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.
2) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

gitcoinbot commented Jul 10, 2019

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 10 months, 2 weeks from now.
Please review their action plans below:

1) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.
2) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.
3) 00nsakes00 has applied to start work (Funders only: approve worker | reject worker).

I'm trying to get into the open-source world. In order to help fix your issue first, I would like to figure all of your problems then I would like to create a design. We would agree on a final design. Finally, I will compile all of this and code you the UI.
4) rikku-x has applied to start work (Funders only: approve worker | reject worker).

Hey,
I have experience with status-react app and have already finished a couple of bounties for Status. Can implement this one too.
5) bitsikka has been approved to start work.

I worked on preview privacy mode and have fairly good understanding of the code base. My plan is to re-examine the code base around profile screens, find an optimal plan to achieve the changes/additions and execute it.

I recall wanting to improve the switch/toggle button in Android which was behaving less than desirably. Visually it was going flip-flop-flip/flop-flip-flop (probably still does) instead of flip/flop. Have to address that issue too in another PR if it is still doing that.

Learn more on the Gitcoin Issue Details page.

@AshleyOyt
Copy link

Hi, Im interested in working on this issue. However, after reading the bounty description, I'm a bit confused as to whether the job is to help design the UI or code the UI or both. If someone could clarify this for me, I would really appreciate it, thanks!

@hesterbruikman
Copy link
Contributor

Hi @AshleyOyt, thanks for your interest! This issue is about coding the UI.

@Rikku-x
Copy link
Contributor

Rikku-x commented Jul 26, 2019

Hey, I have applied on Gitcoin and want to work on this issue.

@bitsikka
Copy link
Contributor

bitsikka commented Aug 11, 2019

Work on #8069 needed to be stable to start work on this.

Starting now

@hesterbruikman
Copy link
Contributor

@bitsikka can you please check in once you're ready to start this work. There are some changes reflected in Figma I think will be helpful for us to go over.

https://www.figma.com/file/bPS9GrvMr7LnH7vnkmuveQfd/Settings?node-id=576%3A2461

@bitsikka
Copy link
Contributor

@bitsikka yeah, making a proper list item component would make you a legend, I've been chasing engineers around with this for as long as I work here :D

I see, challenge accepted :)

I have seen the use of "position: absolute" in places in the name of achieving pixel perfect thereby resulting inflexible components :)

It's covered to some extent by this issue here #7785

I'm sure know Figma by now well enough to find the spec,

yes, this along with descriptions below helps a ton

holla at me on Status if you need any help.

will do

The structure the way I see it is something along this

Just what I was looking for, and along the line of thought I was having - makes sense - thanks!

@flexsurfer
Copy link
Member Author

Proper list item component could be found here https://github.com/status-im/status-react/blob/develop/src/status_im/ui/components/list_item/views.cljs

@bitsikka
Copy link
Contributor

Proper list item component could be found here https://github.com/status-im/status-react/blob/develop/src/status_im/ui/components/list_item/views.cljs

Thanks @flexsurfer! I will make good use of this

@flexsurfer
Copy link
Member Author

btw i've added action here https://github.com/status-im/status-react/pull/8731/files#diff-89291a94b8ecb5fd6c670fe5ea7f7ab8R30, will be merged soon, so if you provide icon and action it will add blue circle

@flexsurfer
Copy link
Member Author

done type - optional :default , :small

  • bool to display the subtitle text, by default it should accommodate any number of lines as needed to fit the text but likely we want an optional number to override it with a limit

done : subtitle `

  • prop to choose what kind of image the list item displays:

done :image or :image-path or :icon

• a regular image with url (think chat avatars)

done :image-path

• icon, the default style is blue fill for the icon and light blue for background

will be merged soon :icon and :active type

• an initial letter (used in group/public chats/tokens etc.), randomised background colour

implemented outside component , provided as : image propertly [chat-icon/custom-icon-view-list (:name token) color 32]

  • prop to control list item's appearance
    • default, black Title
    • button, Accent blue Title
    • disabled, Dark Grey Title, Dark Grey Icon, light grey Icon Background
    • destructive, Red Title, Red Icon, 10% red Icon background

:type - default (:default) , button (:active) - done
disabled,destructive not done

  • optional prop for chat title icons (the grey little icons that go before the chat title)
    • group chat
    • public chat

better to implement outside component and just pass as property

  • boolean to display on/off the right '>' chevron

done
:accessories [ :chevron]

  • prop to change the right side accessory
    • text
    • switch
    • radio button
    • checkbox
    • badge
    • chat accessory (timestamp and badge)

done
; accessories - optional vector of :chevron, :check or component or string

@errorists
Copy link
Contributor

thanks @flexsurfer ! 🤗 guys, once you have some time can I ask you to document it somewhere what's the current state of component implementation and ideally where they're used so we know what needs a refactor (looking at you chat)?

@bitsikka
Copy link
Contributor

bitsikka commented Aug 20, 2019

I'm making good progress on this. Will push WIP PR soon.

btw
@errorists I noticed a significant difference between this

Group 3

and this

Group

While I'm not concerned so much about the latter in Profile area, and I'm going with the spec in the former(this implementation already existed as pointed out by @flexsurfer), in near future it can make a difference.

@flexsurfer
Copy link
Member Author

btw first iteration of fiddle has been merged
https://github.com/status-im/status-react/tree/develop/fiddle
and you can find list-item examples
https://github.com/status-im/status-react/blob/develop/fiddle/src/fiddle/views/list_items.cljs

@flexsurfer
Copy link
Member Author

and this

Group

regarding this , i think it's better to implement it outside list-item component because it's too specific

@bitsikka
Copy link
Contributor

bitsikka commented Aug 20, 2019

btw first iteration of fiddle has been merged
https://github.com/status-im/status-react/tree/develop/fiddle
and you can find list-item examples
https://github.com/status-im/status-react/blob/develop/fiddle/src/fiddle/views/list_items.cljs

@flexsurfer
In the latest rebase I saw you added :more accessory 👍

While It is already getting very close to the desired spec, may I please take the liberty to make subtle adjustments?. I promise to not break existing usage(test it thoroughly) and update fiddle accordingly.

Some things I'm adding

  • :disabled? prop - currently takes advantage of whether on-press is supplied. There are cases, like ENS list-item in Profile where sometimes it needs to be disabled(no registrar or no name) even when on-press is supplied.
  • on-long-press prop
  • :action-destructive theme
  • :default theme to have blue icon with blue background and black :title which is common like in my Profile view, as opposed to :action theme which has blue icon and blue :title
  • :disabled? prop also makes all themed list-item grayed out(:title, :icon)
  • ...

@errorists
Copy link
Contributor

errorists commented Aug 20, 2019

I noticed a significant difference between this and this

@bitsikka thanks for noticing, 10 is actually the correct value. 12 would be more logical since the value would be shared with the circle icon but that places both text labels too close to each other for comfort.

@errorists
Copy link
Contributor

some other comments, I leave that to you guys if touchable behaviour should be a part of the component (it's always the same for all list items), the correct implementation of that is touchableHighlight to grey as Andrey did in the '+ button' bottom sheet.

:default theme to have blue icon with blue background and black :title which is common like in my Profile view, as opposed to :action theme which has blue icon and blue :title

I agree it makes sense, blue text is used specifically to call out action buttons in the interface.

@bitsikka
Copy link
Contributor

bitsikka commented Aug 20, 2019

I noticed a significant difference between this and this

@bitsikka thanks for noticing, 10 is actually the correct value. 12 would be more logical since the value would be shared with the circle icon but that places both text labels too close to each other for comfort.

Thanks for clearing that up 👍 I think spec with 10 can make it quite flexible and it is the right choice

If I can make the chat list item covered by the same list item, would you please consider it? cc @flexsurfer

  • I was thinking to have a prop :title-prefix which could either be string, keyword representing tiny-icon, or component(additional flexibility.
  • :title row would then also optionally have :title-row-accessory as suffix which would hold time-stamp component.
  • :subtitle row would optionally be a string or component(for chat list which needs to show text or emoji or command or gif... etc) and
  • :subtitle row would have the unread-count badge as suffix

If we have this, we can use it in places like wallet Asset list where :title-prefix would be a string

  • we would need :title-color-override to gray out the :title part

This way the list item will have additional versatility + design is constrained and consistent everywhere

@flexsurfer
Copy link
Member Author

some other comments, I leave that to you guys if touchable behaviour should be a part of the component (it's always the same for all list items), the correct implementation of that is touchableHighlight to grey as Andrey did in the '+ button' bottom sheet.

this is another difference we use touchableOpacity in web version, so it will be different from RN : ( but in RN we need to implement proper version for sure

@flexsurfer
Copy link
Member Author

@bitsikka i wouldn't use list-item for chat items, because it'll make list-item too complex, and chat item is too specific and used only in one place, so it's better to develop a separate component for chat-list item

@errorists
Copy link
Contributor

I was thinking to have a prop :title-prefix which could either be string, keyword representing tiny-icon, or component(additional flexibility.

That would be awesome, definitely we'd like to have that possibility since we use it, for example we have those 'new' badges for new features like when we enable Tribute to Talk in profile for example.
Screenshot 2019-08-20 at 08 30 13

I don't have anything to add on the chat item as list-item component or separate discussion you guys are having. In Figma it was easier for us and more logical to treat it as as part of the same component only with its own dedicated nested objects we could toggle on/off. I don't think there's a right or wrong way of doing that, I leave that to you guys :)

@bitsikka
Copy link
Contributor

@bitsikka i wouldn't use list-item for chat items, because it'll make list-item too complex, and chat item is too specific and used only in one place, so it's better to develop a separate component for chat-list item

@flexsurfer
Sounds good 👍 I understand with :content component in title/subtitle area feature that already exists is simple and works 👍 just as well

@flexsurfer
Copy link
Member Author

flexsurfer commented Aug 20, 2019

:title-prefix looks good, in that case we could provide accessories as custom view with time and messages counter for chat

image

@bitsikka
Copy link
Contributor

I was thinking to have a prop :title-prefix which could either be string, keyword representing tiny-icon, or component(additional flexibility.

That would be awesome, definitely we'd like to have that possibility since we use it, for example we have those 'new' badges for new features like when we enable Tribute to Talk in profile for example.
Screenshot 2019-08-20 at 08 30 13

I don't have anything to add on the chat item as list-item component or separate discussion you guys are having. In Figma it was easier for us and more logical to treat it as as part of the same component only with its own dedicated nested objects we could toggle on/off. I don't think there's a right or wrong way of doing that, I leave that to you guys :)

sounds good 👍 will make the :title-prefix happen

@bitsikka
Copy link
Contributor

Wonderful! Thanks @flexsurfer @errorists for clearing things up. I will make a WIP PR later today.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 800.0 DAI (800.0 USD @ $1.0/DAI) has been submitted by:

  1. @bitsikka

@StatusSceptre please take a look at the submitted work:


@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


The funding of 800.0 DAI (800.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @bitsikka.

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

Successfully merging a pull request may close this issue.