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

Settings: Move settings to it's own tab #528

Closed
luke-freeman opened this issue Nov 6, 2020 · 12 comments · Fixed by #539 or #561
Closed

Settings: Move settings to it's own tab #528

luke-freeman opened this issue Nov 6, 2020 · 12 comments · Fixed by #539 or #561
Labels
enhancement New feature or request

Comments

@luke-freeman
Copy link

Settings currently lives under 'My Tutorials'. Move Settings to it's own top level tab

Expected behavior

Settings has it's own tab

Screenshot 2020-11-06 at 16 26 17

Actual behavior

Settings can only be accessed from My Tutorials

Screenshot 2020-11-06 at 16 29 14

@luke-freeman luke-freeman added the enhancement New feature or request label Nov 6, 2020
@byaruhaf
Copy link
Contributor

byaruhaf commented Nov 11, 2020

@luke-freeman, I'm trying to implement this enhancement wanted your advice on three items

  1. Move Settings to it's own top level tab, does that mean that we remove Settings from My Tutorials
  2. If 1 above is true then X in the setting screen is no longer needed since switching away from settings will done on the tab bar.
  3. is there a Design for Settings with a tab bar the only one I see in Figma is setting with out a tab bar.
Image GIF
IMG_9EB8E67C67C1-1 RPReplay_Final1605123387

@luke-freeman
Copy link
Author

@byaruhaf

  • That's right it should no longer exist on 'My Tutorials'
  • There's no design but can you set it up as if it wasn't a modal and instead a typical view? (So no x)

@luke-freeman
Copy link
Author

@byaruhaf @0xTim

Functionally this works great!

But the new settings icon is misaligned. Can you align it to the top with the other icons?

Screenshot 2020-12-17 at 10 13 41

@byaruhaf
Copy link
Contributor

@luke-freeman I have updated the setting icon is this ok.

Simulator Screen Shot - iPhone 12 Pro Max - 2020-12-17 at 12 35 53

@luke-freeman
Copy link
Author

luke-freeman commented Dec 17, 2020

Screenshot 2020-12-17 at 11 57 46

@byaruhaf

Now the settings icon is too big. It should be the same size as the others and sit comfortably between the lines in the screenshot above

@byaruhaf
Copy link
Contributor

@luke-freeman I have shrunk the icon, please check two options below:

Option 1
23

Option 2
21

@luke-freeman
Copy link
Author

@byaruhaf

I think the settings icon needs to be 1-2pt/px bigger.

Also I see some new issues:

  • Is it just me or the settings icon appears blurry now?
  • Also the settings icon is using a different grey to the other icons

@bladebunny
Copy link
Contributor

PR submitted to match existing icons.
#559

@byaruhaf
Copy link
Contributor

byaruhaf commented Apr 7, 2021

Hi @luke-freeman

Have you taken a look at the settings icons in PR #561? They look as shown below:
if this is ok then we can close this issue.
if it's not ok can you design the setting icon the same as the library icon & library icon for this use case

Simulator Screen Shot - iPhone 12 Pro Max - 2021-04-07 at 04 43 54

@bladebunny
Copy link
Contributor

bladebunny commented Apr 7, 2021

For some reason this isn't the most recent. Here's what at least the PR for the settings icon itself looks like. (Lines added in Dark Mode). I can switch out the image if need be. The issue was the first set of icons use actual images whereas the settings icon was using a font image (systemName) -- where the final size and internal padding vary. I would argue where precision counts you probably don't want to use the system font images. I would prefer svg images or png's.

Simulator Screen Shot - iPhone 12 Pro - 2021-04-07 at 12 02 19
Simulator Screen Shot - iPhone 12 Pro - 2021-04-07 at 12 02 40

@byaruhaf
Copy link
Contributor

byaruhaf commented Apr 8, 2021

This looks good, lets wait for @luke-freeman comment

@luke-freeman
Copy link
Author

@byaruhaf @bladebunny

Looks excellent thanks!

@0xTim 0xTim linked a pull request Apr 8, 2021 that will close this issue
@0xTim 0xTim closed this as completed in #561 Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants