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.

Sign up for GitHub

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

Jump to bottom

Feat/54 add an introduction textarea #932

Merged
merged 3 commits into from
Apr 23, 2024
Merged

Feat/54 add an introduction textarea #932

merged 3 commits into from
Apr 23, 2024

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Mar 15, 2024 •
edited
Loading

Summary

introduce a description editor at the top similar to Files

Screenshots:

Viewing a Table:
image

Creating a Table:
image

Editing a Table:
image

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@grnd-alt grnd-alt linked an issue Mar 15, 2024 that may be closed by this pull request
Add an introduction textarea #54
Closed
@grnd-alt grnd-alt changed the title Feat/54 add an introduction textarea WIP: Feat/54 add an introduction textarea Mar 15, 2024
@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from dc1b1f9 to 052da12 Compare March 15, 2024 09:39
@juliusknorr
Copy link
Member

Very nice, two thoughts that might be worth to discuss with @nextcloud/designers :

  • The editor is not collaborative, so maybe we want to make it read only by default with a button to toggle to edit
  • From my perspective it would be nicer to left-align as we do it in the files app with the folder description

@enjeck
Copy link
Contributor

enjeck commented Mar 18, 2024

@grnd-alt I think we're supposed to have this introduction textarea in both the "Create table" and "Edit table" modals. Similarly to the way we have the "Title".

@juliushaertl said:

The editor is not collaborative, so maybe we want to make it read only by default with a button to toggle to edit

Yes, the editor should be read-only when viewing a table. But we don't need a button to toggle edit, imo. We should handle this in the "Edit table" modal instead.

@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch 2 times, most recently from 3b10e16 to 34b8b05 Compare March 19, 2024 09:56
jancborchardt
jancborchardt reviewed Mar 19, 2024
View reviewed changes
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.

  • The editor is not collaborative, so maybe we want to make it read only by default with a button to toggle to edit
  • From my perspective it would be nicer to left-align as we do it in the files app with the folder description

Agree with both of @juliushaertl’s points. Actually a great example of how this could look is the Forms app:

View mode

image

Edit mode

image

@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from 74e7a71 to f87357f Compare March 25, 2024 12:55
@grnd-alt
Copy link
Member Author

@jancborchardt
Thanks for you're input about the way it's done in forms.
I've update the screenshots with the new layout. I feel like it could still be improved. It's all quite clustered to the left and the right feels empty. But I don't think we can move the Edit/View/Hide buttons on the same line as the title as they only effect the description editor and not the entire table. I'd propose moving it back to aligning it in the middle (Screenshot Option 2). But if it's better to have it at the left, or you have a different idea I'll happily adapt it.

jancborchardt
jancborchardt requested changes Mar 26, 2024
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@grnd-alt nice, thanks for both the suggestions! :)

  • A little bit of moving the description can already achieve a lot, like below when the text is left-aligned with the text of the table below (see modified screenshot below)
  • The radio button is a bit huge there and not super needed. I would say an "Edit"/"Done" button in the top right like we have in Collectives is better in this case.
  • Is "Hide" a requirement? If so, we can do that via a simple collapsing arrow like we do in Collectives too for the "Recent pages" section
  • Can we somehow hide the "Show outline" button in view mode, it is not needed here. cc @juliushaertl

316519952-a2efb4bc-ee82-44fd-aa80-f4408ccef8ed

@grnd-alt grnd-alt changed the title WIP: Feat/54 add an introduction textarea Feat/54 add an introduction textarea Mar 27, 2024
jancborchardt
jancborchardt approved these changes Mar 27, 2024
View reviewed changes
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.

Seems good to me! Not 100% sure about the placement of the "Edit" button but there’s so many options that I think just getting it in now is better. :) @juliushaertl?

juliusknorr
juliusknorr requested changes Mar 27, 2024
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Small comments, otherwise this looks good 👍

lib/Command/RenameTable.php Outdated Show resolved Hide resolved
src/modules/main/sections/TableDescription.vue Outdated Show resolved Hide resolved
src/modules/main/sections/TableDescription.vue Outdated Show resolved Hide resolved
@juliusknorr juliusknorr added enhancement New feature or request 2. developing Work in progress labels Mar 27, 2024
@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from 308b5bb to f890601 Compare March 28, 2024 08:51
@enjeck
Copy link
Contributor

enjeck commented Apr 1, 2024

@jancborchardt I find it very confusing to have an edit button just for the single "description" field. Then a separate button that edits the title in a separate place. Then no way to add a description when creating a table. As a user, I would expect to be able to add a description when creating a table. And to either have a single button to edit everything at once or to be able to inline edit everything. Having a mix feels weird. Maybe it's just me

@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from f890601 to 22b79ac Compare April 2, 2024 09:23
@jancborchardt
Copy link
Member

I find it very confusing to have an edit button just for the single "description" field. Then a separate button that edits the title in a separate place. Then no way to add a description when creating a table. As a user, I would expect to be able to add a description when creating a table. And to either have a single button to edit everything at once or to be able to inline edit everything. Having a mix feels weird. Maybe it's just me

@enjeck yeah you are right – so then should we move the description box editing (and adding) into the modal? And on the tables page we show it read-only, just like we do for the title?

@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from 22b79ac to 69a54c4 Compare April 2, 2024 10:00
@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch 2 times, most recently from ac14dfa to 1f87bac Compare April 10, 2024 09:02
enjeck
enjeck reviewed Apr 16, 2024
View reviewed changes
src/store/store.js Outdated Show resolved Hide resolved
@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch 2 times, most recently from 2fd4ce9 to 4472313 Compare April 17, 2024 12:32
@grnd-alt
add description to Tables
1f6e895 
Signed-off-by: grnd-alt <salimbelakkaf@outlook.de>
@grnd-alt
add TableDescription To Context
45dafe6 
move Table Description to EditTable

add tables Description to createTable

Signed-off-by: grnd-alt <salimbelakkaf@outlook.de>
@grnd-alt
adapt cypress test for tables description
0febcbe 
Signed-off-by: grnd-alt <salimbelakkaf@outlook.de>
@grnd-alt grnd-alt force-pushed the feat/54-add-an-introduction-textarea branch from 970e045 to 0febcbe Compare April 17, 2024 13:15
blizzz
blizzz approved these changes Apr 17, 2024
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

for the PHP bits :)

@enjeck
Copy link
Contributor

enjeck commented Apr 23, 2024

This looks good to me too. Thanks, @grnd-alt

@blizzz
Copy link
Member

blizzz commented Apr 23, 2024

@juliushaertl your change request is still active

@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Apr 23, 2024
@juliusknorr juliusknorr merged commit 6822b9e into main Apr 23, 2024
57 checks passed
@juliusknorr juliusknorr deleted the feat/54-add-an-introduction-textarea branch April 23, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enjeck enjeck enjeck left review comments

@juliusknorr juliusknorr juliusknorr approved these changes

@blizzz blizzz blizzz approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add an introduction textarea
5 participants
@grnd-alt @juliusknorr @enjeck @jancborchardt @blizzz