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

Global settings priorities discussion #909

Closed
8 tasks done
melissaperreault opened this issue Nov 17, 2021 · 9 comments
Closed
8 tasks done

Global settings priorities discussion #909

melissaperreault opened this issue Nov 17, 2021 · 9 comments
Assignees

Comments

@melissaperreault
Copy link
Contributor

melissaperreault commented Nov 17, 2021

Context

After prototyping and regrouping we've come to a space where we can talk about priorities divide and conquer.
Below I will list what comes to mind, let's combined and divide on multiples issues after and moving forward. Just feels right to have everything at one place to see what we have to do.

Meeting MVP expectations

  • 1. Remove from the demo store reference (if not done yet) the following settings categories to align with the latest decision
    - Sections
    - Dividers and lines
  • 2. By removing Sections settings, 2 things we need to introduce
    - Image with text section, the text part would be styled by the Text box setting moving forward and the media will renamed managed by Media setting
    - Bring Vertical space between sections setting to Layout category (Formerly named Grid) (Preview of the new hierarchy internal file)
  • 3. Split Popups and Drawers to 2 distinct categories and follow the Figma guidance
  • 4. Discuss if setting combination conditions should be done in a single PR or more granularly
    - Examples:
    - Do not duplicate border for touching borders when horizontal spacing is set to 0
    - Remove the corner radius when horizontal spacing is set to zero or when the component hug another one.

Follow up tasks

  • Buttons review feedback, I can see where ensuring section styles behave as expected will have a greater impact for this week
  • Prioritize setting's feedback for each setting categories
  • Define specific range of each setting variables
  • Content update, this can come in next week once we (UX) have more guidance to give for official settings values and condition rules for all items. I could own and contribute to content update!
@KaichenWang
Copy link
Contributor

KaichenWang commented Nov 18, 2021

@tyleralsbury's notes:

there are a lot of broader topics around systemic rules that need to be considered
Some of the things being considered are:

  • Borders collapsing when elements are flush
  • Shadows on buttons adding to the padding between them to stop weird overlaps
  • Drawers losing their borders / corner roundness on the sides that touch the viewport

These are rules that would be applied consistently throughout the theme - they are different from the dreaded "magic" in that they are 100% consistent. There is no "sometimes" in these circumstances, so they are forming rules as opposed to breaking them.

@tyleralsbury
Copy link
Contributor

Remove from the demo store reference (if not done yet) the following settings categories to align with the latest decision
Sections
Dividers and lines

Dividers and lines are already gone, but not Sections.

@kmeleta
Copy link
Contributor

kmeleta commented Nov 18, 2021

Those priorities seems reasonable to me. Just some random remarks on some of them..

  • Image with text updates should be a relatively quick win I'd think.

  • For vertical spacing between sections, moving the settings to layout should be quick enough, but I'd love if we had time to solve this more globally than previously done in prototypes. Remove section-specific margin overrides where possible, make title markup and margins predictable/consistent across sections, etc. I feel a little more time spent to clean this up now could save time/bugs later.

  • Not sure if Kai already has thoughts on the combination conditions but we definitely need give some thought/explore as to how we make elements "spatially aware". The approach likely may just have to be more granular and specific to each section, but it'd be nice to see if there are any global rules we can establish somehow.

  • Before hearing our current priorities, I did start looking at button/input feedback to clean up the shadow weirdness and explore how we may include some built in padding to shadows don't overlap with others. I can pause on this for now, but wanted to share that it was in progress.

@KaichenWang
Copy link
Contributor

KaichenWang commented Nov 18, 2021

I think before we start any development, there's some documentation housekeeping to complete:

  • Cross-reference the settings doc’s tables with the updates that @melissaperreault outlined. For example the “Vertical space between sections” which is currently under “Descoped - Sections” would need to be moved under “Layout”
  • Cross-reference the finalized doc’s tables with what’s currently on all-global-settings branch and identify which settings need to be removed + added
  • Add test scenarios to the document. For each setting group, identify key areas/edge cases to test. I think it will help when we start making the changes to ensure we’re not missing any use cases. For example, for buttons:

Screen Shot 2021-11-18 at 11 50 03 AM

Once dev and design has signed off on the finalized doc, we can start the implementation knowing that we all aligned on what settings need to be changed and how to test for them

@melissaperreault
Copy link
Contributor Author

melissaperreault commented Nov 18, 2021

Additional change to consider in our list

  • When Full width section setting occur, ignore corner radius. (I think we do already)
  • Text boxes; we are removing the Padding setting, we'll need to revert to the initial section padding

@melissaperreault
Copy link
Contributor Author

Cross-reference the settings doc’s tables with the updates that @melissaperreault outlined. For example the “Vertical space between sections” which is currently under “Descoped - Sections” would need to be moved under “Layout”

✅ This is done already! 🎉

@kmeleta
Copy link
Contributor

kmeleta commented Nov 18, 2021

  • When Full width section setting occur, ignore corner radius. (I think we do already)

I know we do for corner radius set via sections settings (which are about to be removed). I'm not sure if we do this for textboxes/media/whatever in sections that can go fullwidth. I'd venture a guess we mostly don't.

@KaichenWang
Copy link
Contributor

KaichenWang commented Nov 18, 2021

Developer action items

@kmeleta

  • Revert section settings and move vertical spacing under layout settings
  • Remove padding from text boxes settings
  • Split Popups and Drawers to 2 distinct categories and follow the Figma guidance
  • Fix shadows on buttons adding to the padding between them to stop weird overlaps

@KaichenWang

Also, while working on settings, identify and add test cases (sections/elements affected by settings + edge cases) to the settings doc

@melissaperreault
Copy link
Contributor Author

Fix shadows on buttons adding to the padding between them to stop weird overlaps

This we won't need to do since we'll rely on a margins setting to allow this flexibility.
Closing this issue since we've completed the essential.

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

No branches or pull requests

4 participants