Skip to content

show card details in modal - #2047

Merged
juliusknorr merged 10 commits into
masterfrom
enh/cardDetailsInExt
Sep 1, 2020
Merged

show card details in modal#2047
juliusknorr merged 10 commits into
masterfrom
enh/cardDetailsInExt

Conversation

@jakobroehrl

@jakobroehrljakobroehrl commented Jun 19, 2020

Copy link
Copy Markdown
Contributor

Signed-off-by: Jakob Röhrl jakob.roehrl@web.de

  • Target version: master

grafik
grafik
grafik

@stefan-niedermann

Copy link
Copy Markdown
Member

May i ask what the intention of this modal is?
Is it to enable a user to edit distraction-free?
Because if the intention is to give the user more space to edit and view the description - it's not that much difference yet 😉

@jakobroehrl

Copy link
Copy Markdown
ContributorAuthor

May i ask what the intention of this modal is?
Is it to enable a user to edit distraction-free?
Because if the intention is to give the user more space to edit and view the description - it's not that much difference yet 😉

Sure!

  • more space and distraction-free editing
  • use it for showing the content of archived cards

@jakobroehrl
jakobroehrlforce-pushed the enh/cardDetailsInExt branch from a6037e2 to 43af8b0CompareJune 23, 2020 09:19
@juliusknorr

juliusknorr commented Jun 23, 2020

Copy link
Copy Markdown
Member
  • Modal text should not be centered - I assume there is a to wide css rule somewhere
  • Check how multiple modals behave e.g. when adding an attachment
  • Move the action to the sidebar actions menu
  • The sidebar should not be shown if the modal is used (open it directly from the board view)
  • Persist state of cardDetailsInModal in browser local storage
  • Show activity/attachments/comments in the modal. This is especially required for showing archived cards
  • description, users and tags are not updated in the sidebar
  • Add a button to the modal to switch back to the sidebar view
  • Fix styling in the modal view, not sure if this works properly with just reusing the sidebar view or of we need to come up with our own tab implementation, but the first should of course be preferred
  • In a second step we could allow users to use the modal as a default opener

@jakobroehrl
jakobroehrlforce-pushed the enh/cardDetailsInExt branch 2 times, most recently from 1f8e9f4 to f2f869eCompareJune 24, 2020 09:41
@juliusknorr

Copy link
Copy Markdown
Member

@jakobroehrl I took the liberty to push two commits that I came up with when trying out your branch. This basically makes sure that the modal can be properly reopened and in the same run adds a checkbox to toggle between default open for the sidebar and default open the modal. I've added another TODO to the above list for persisting that state.

@jakobroehrl

Copy link
Copy Markdown
ContributorAuthor

Fix styling in the modal view, not sure if this works properly with just reusing the sidebar view or of we need to come up with our own tab implementation, but the first should of course be preferred

@juliushaertl To do this, we need to remove the following CSS style from the sidebar: width, max-width, border-left
Is this possible?

@juliusknorr

Copy link
Copy Markdown
Member

@juliushaertl To do this, we need to remove the following CSS style from the sidebar: width, max-width, border-left Is this possible?

Yes, i think for that we can just overwrite the styles with a v-deep selector in in the modal.

@jakobroehrl
jakobroehrlforce-pushed the enh/cardDetailsInExt branch from 4e1cc09 to f93be51CompareJuly 10, 2020 08:03
@jakobroehrl

Copy link
Copy Markdown
ContributorAuthor

@juliushaertl
This does work if I add it here:

deck/src/App.vue

Lines 111 to 115 in f93be51

.app-sidebar::v-deep {
border-left: 0;
width: 800px;
max-width: 780px;
top: 0px;

But it affects all sidebars as well, how can we change this?

@jakobroehrl
jakobroehrlforce-pushed the enh/cardDetailsInExt branch 2 times, most recently from 56342f6 to 5b6f8b6CompareJuly 17, 2020 07:33
@juliusknorr

Copy link
Copy Markdown
Member

Pushed the rebased version and removed the for now unneeded details tab component. Let's keep the changes minimal for now.

@jakobroehrl Mabye you can have another look at the modal styling, the scrollbar looks out of place:

image

@jakobroehrl

Copy link
Copy Markdown
ContributorAuthor

@juliushaertl
Thanks for your commit, let's merge this PR!

@juliusknorrjuliusknorr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes are good to go from my side now 👍

@jakobroehrl Mind to make sure that CI passes? 😉

jakobroehrland others added 6 commits September 1, 2020 09:41
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
juliusknorrand others added 4 commits September 1, 2020 09:41
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorr merged commit d432a07 into masterSep 1, 2020
@juliusknorr
juliusknorr deleted the enh/cardDetailsInExt branch September 1, 2020 08:10
@juliusknorrjuliusknorr mentioned this pull request Sep 1, 2020
7 tasks
Sign up for freeto 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.

3 participants

@jakobroehrl@stefan-niedermann@juliusknorr