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

Apply grid layout to deck options 2 #1212

Closed
wants to merge 4 commits into from

Conversation

kleinerpirat
Copy link
Contributor

@kleinerpirat kleinerpirat commented May 31, 2021

Closes #1210.

This is a less intrusive version of my previous PR. I have split GeneralOptions.svelte into

  • TimerOptions.svelte
  • AudioOptions.svelte

for more consistency.

The grid layout is now better encapsulated in ConfigEditor.svelte.


Other changes

  • reduce <h2> font size to 1.5em
  • adjust padding of #main and .outer

Showcase


image


I would highly appreciate if someone could tell me how to run Prettier on checkout via VS-Code.

@hgiesel
Copy link
Contributor

hgiesel commented May 31, 2021

I would highly appreciate if someone could tell me how to run Prettier on checkout via VS-Code.

bazel run //ts:format

To execute them as part of a hook, you could add them to e.g. .git/hooks/pre-commit.

@kleinerpirat
Copy link
Contributor Author

kleinerpirat commented May 31, 2021

Thanks, Henrik!

I thought about using a Svelte prop rows instead of the two classes small and large, but that would require changes within the individual option components (similar to the previous PR).

Is eb03d8e a viable option or would you prefer a more dynamic or completely different approach? I don't mind if anyone (looking at you @hgiesel) creates an alternative since I'm just starting out here.

Just curious what you think about the grid layout in general.

@hgiesel
Copy link
Contributor

hgiesel commented May 31, 2021

First of all, I think there's some duplicated effort here with #1207, e.g. splitting up TimerOptions and AudioOptions.

In this PR, I also made another suggestion regarding the issue of wider windows, which was to have a scrollspy / navigation element to the left of the main content, taking inspiration from other configuration menus, like the Chrome preferences.

Having the content collapse into columns is a good way to prevent overly wide input elements, however you still have to scan the entire page to find the appropriate section.

In the end, it will be up to @dae to make a judgement :)

@kleinerpirat
Copy link
Contributor Author

Ah yes, glad you pointed that out. I wasn't aware of your refactoring efforts when I worked on these changes. I'm happy to postpone design contributions such as this one until the foundations are more solid / reworked.

The last thing I want is for you guys to have more work because of me - that would be the opposite of what's intended ;)

@dae dae mentioned this pull request Jun 2, 2021
2 tasks
@kleinerpirat
Copy link
Contributor Author

I'll close this PR and create a new one after #1207 gets merged.

The double column-design I suggested on the forum would only need a few minor additions to DeckOptionsPage.svelte.

@dae
Copy link
Member

dae commented Jun 2, 2021

Thanks Matthias :-)

@kleinerpirat kleinerpirat deleted the grid-options-2 branch July 28, 2022 12:10
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.

None yet

3 participants