Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Aug 1, 2022

image

TODOs:

  • Implement saving
  • Admin settings
  • Check with the screen reader to confirm that the accessibility improved
  • Remove old php templates
  • Make cypress tests work

@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Aug 1, 2022
@CarlSchwan CarlSchwan self-assigned this Aug 1, 2022
@CarlSchwan CarlSchwan marked this pull request as draft August 1, 2022 14:02
@CarlSchwan
Copy link
Member Author

@szaimen pointed out to me #589, I'll try to merge both efforts

@CarlSchwan CarlSchwan requested a review from artonge August 1, 2022 18:20
@CarlSchwan CarlSchwan changed the title WIP: Port settings to vue Port settings to vue Aug 1, 2022
@CarlSchwan CarlSchwan marked this pull request as ready for review August 1, 2022 21:53
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice! Only details:

  • Can you move the Mail/Push checkbox columns to the left again, so there’s not so much whitespace and it’s all nicely left-aligned like before? See screenshot below
  • I can’t fully tell from the code – are the "Files", "Calendar, contacts and tasks" and "Other activities" marked up as h3?

image

@nimishavijay
Copy link
Member

Can you move the Mail/Push checkbox columns to the left again, so there’s not so much whitespace and it’s all nicely left-aligned like before? See screenshot below

I actually like it how @CarlSchwan did it, before the text alignment was a bit messy.
What do you think about keeping it like this but decreasing the space between the text and checkboxes? @jancborchardt

@CarlSchwan
Copy link
Member Author

Nice! Only details:

* Can you move the Mail/Push checkbox columns to the left again, so there’s not so much whitespace and it’s all nicely left-aligned like before? See screenshot below

My main motivation to moving the checkbox to the right was to simplify the layout for screenreader (we don't have this two level of headers) and since the content is read from the left to the right the screenreader will first read the description of the checkbox and then tell the state of the mail notification and push notification for each row.

This also solves some alignment issues with the content not at all left aligned. See screenshot

align

* I can’t fully tell from the code – are the "Files", "Calendar, contacts and tasks" and "Other activities" marked up as h3?

Done, this is now using h3

@jancborchardt
Copy link
Member

Cool, sounds good @CarlSchwan – do you mind decreasing the space between text and checkboxes a bit like @nimishavijay proposed? :)

@CarlSchwan CarlSchwan force-pushed the port/settings-vue branch 2 times, most recently from 42a25ab to f344fc8 Compare August 16, 2022 14:15
@CarlSchwan
Copy link
Member Author

I still need one approval ;)

Signed-off-by: Carl Schwan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants