deck
deck copied to clipboard
feat: #2906 add card ID badge
- adds vscode settings file to gitingore
- adds new badge for card ID
- adds card ID to board filter
- adds settings to disable card ID badge
- Resolves: #2906
- Target version: master
Summary
Adds ID Badge to top left corner of card.
Checklist
- [x] Code is properly formatted
- [x] Sign-off message is added to all commits
- [x] Tests (unit, integration, api and/or acceptance) are included
- [x] Documentation (manuals or wiki) has been updated or is not required
@juliushaertl can this be marked as to review please?
I'd say we can add it to the list of card badges on the bottom left, but would also appreciate feedback from @nimishavijay and @jancborchardt about that and how we should handle this in terms of the extra setting for showing/hiding the number. Note that the id is unique across users/boards so on larger instances this might quickly become a number with >10 digits
@juliushaertl that is why I placed it above the title, already tested with up to 12 digits and it works without impact. @stefan-niedermann I'd appreciate design input, see this as a technical draft, happy to edit to fit in. I felt an additional badge took too much space in the grand scheme, so I looked at how Jira and Trello do it. @juliushaertl setting for hiding this feature is already implemented, I am unsure of what you mean.
@juliushaertl that is why I placed it above the title, already tested with up to 12 digits and it works without impact.
How does it help a user to quickly see the 12 digit id of a card? That's not really a number you memorize for refering to a card in a conversation, no?
@juliushaertl that is why I placed it above the title, already tested with up to 12 digits and it works without impact.
How does it help a user to quickly see the 12 digit id of a card? That's not really a number you memorize for refering to a card in a conversation, no?
Good point @raimund-schluessler, which @stefan-niedermann also related to in the issue of this PR last March.
However having a possible max of 19 digits (max of BIGINT which is I assume used as DB field type) in the first place is more a design flaw than an issue of this PR itself. Each card should imho have a proper uid
in the first place.
If we look at other popular tools:
- Jira does this with adding project ID and a auto-incrementing number before each ticket Example: https://jira.example.com/browse/OWSWS-422 -> ID = OWSWS-422
- Trello has this at the beginning of the unique identifier in the URL Example: https://trello.com/c/[boardid]/5893-weddingvideo-n2 -> ID = 5893
So their card numbering is depending on the board and thus gets a global proper uid
rather than a global id
value. Making the assumption that a board itself does not accumulate a 12 digit amount of cards over time.
However, I did think about that issue with bigger numbers and also users who don't want this feature in the first place as it might not fit into their workflow. And this is precisely why I added a setting to enable it manually. So in everyones best interest we could define that it is disabled by default, so it doesn't impact anyone's current setup and those who need it can enable it.
Super nice work! :)
I'd say we can add it to the list of card badges on the bottom left
This seems like a good idea! It could go in the beginning of the badges, something like
ID = OWSWS-422
This kind of ID seems really technical. Deck is usually compared with Trello, so a simpler ID like that seems more friendly. Currently all cards have unique IDs across all boards. How about cards within one board have unique IDs? For eg. Right now:
- Board1 has cards with ID 1, 2, 3, 4, 5
- Board2 has cards with ID 6, 7, 8
Instead we could do
- Board1 has cards with ID 1, 2, 3, 4, 5
- Board2 has cards with ID 1, 2, 3
This wouldn't be the same as the ID in the URL, but more like a reference number for each card. It also makes sense that when you create a new board and add a card it shows #1
. What do you think?
So in everyones best interest we could define that it is disabled by default, so it doesn't impact anyone's current setup and those who need it can enable it.
Agreed, there is an addon in Trello for this and it seems quite popular too, so it is okay to have a setting for this since it's a feature that would be useful to power users but maybe not so much to just everyday users.
@jancborchardt any thoughts about this?
4 days later - How are we going to proceed here?
@jancborchardt, any updates from your end?
Hi, sorry @oneWaveAdrian :+1:’d @nimishavijay’s proposal but let me expand:
- Agree with @juliushaertl’s proposal of putting it on the bottom row, and @nimishavijay’s mockup for that is great
- Agree with your assessment @oneWaveAdrian that it should be a setting disabled by default
However, there’s the big confusion about the usefulness of the ID as @juliushaertl mentioned:
Note that the id is unique across users/boards
So this is quite strange, and isn’t that a possible privacy problem too? I would agree with @nimishavijay that if we do show IDs in the UI, they should start at #1
for each board.
However, there’s the big confusion about the usefulness of the ID as @juliushaertl mentioned:
Note that the id is unique across users/boards
@jancborchardt I get the idea behind this: you can move a card between boards and external links to the card still work. So the card can be found even if, let's say it moves from the "User feedback" board to the "In development" board.
Hence my suggestion to add a local ID (as @nimishavijay suggested starting with #1
) and still have the global ID. Therefore nothing it will remain backwards compatible and nothing changes for those users who won't need it.
@nimishavijay you mentioned placing it at the beginning of the badges. What happens if an ID on a boarddoes get up to let's say #9999
? Will that still work design wise/is there enough space?
What happens if an ID on a boarddoes get up to let's say #9999? Will that still work design wise/is there enough space?
Well, in the worst case when the entire bottom row is filled out there would be the card ID, description badge, attachment badge, assignees, and 3dot menu. If there is no space to show all these, we could start hiding the description and attachment badges, and if the ID is so big that there's really no space even after that it should be okay if we add a row below the ID and move the badges, assignees and 3dot menu there (although if the IDs are that large it would not be of much use anyway like @raimund-schluessler mentioned)
Thanks for the design feedback @nimishavijay . So, @juliushaertl, @stefan-niedermann, @raimund-schluessler what are the next steps here in your opinion? Can the card Model be altered to accommodate a local ID? This is more than just a frontend change, but rather a change in card database architecture, therefore I would like some discourse/agreements before starting to alter things and then the change being rejected.
one Month later - what's the decision on this @juliushaertl @jancborchardt @nimishavijay @stefan-niedermann?
@oneWaveAdrian I am not in the position to "make a decision". I am a volunteer contributor and maintainer of the Deck Android app. I can give input and advice, but every else is up to @juliushaertl
As another PR has popped up trying to provide the same feature, this seems to be something a lot of users need/want, can we move on with this topic? Even if you decide to close it, leaving it hanging mid-air forever is not an option to work with your community @juliushaertl @nimishavijay @jancborchardt
Thinking about this a bit more I'd say even the global unique id is fine, as we also expose it for files with internal links. This might especially be useful if we introduce automatic references as GitHub has them with https://github.com/nextcloud/server/issues/31667, so I'd be fine to merge this with the id badge being moved to the bottom row.
That being said, from a user perspective I could see that a local id might still be useful, but I'd consider that a follow up feature. As you mention it would require a separate field being added to the database to expose a unique id per card and would require careful handling for cases where cards are moved across boards.
@juliushaertl Thanks for the feedback. Just to be sure: I am placing the ID in the bottom left as suggested by @nimishavijay, even though this could create visual problems if the ID was too long, correct?
I'd say we can add it to the list of card badges on the bottom left
This seems like a good idea! It could go in the beginning of the badges, something like
Is this a release candidate now? I need it :)
I am placing the ID in the bottom left as suggested by @nimishavijay, even though this could create visual problems if the ID was too long, correct?
Yes, let's go with that I'd say. The code seems good otherwise, so we could merge this if you move the CardId to the CardBadges component. đź‘Ť
@juliushaertl accidentally closed this PR in an attempt to rebase... can you reopen it please? I usually use gitlab and got confused here...
I can't it seems as the branch does not contain any other commits compared to master. Maybe you can reset the branch to one of the previous commit ids: https://github.com/nextcloud/deck/commit/bfb35a506779d6d87ef811e3bb11965f9e3a1e66
@juliushaertl @oneWaveAdrian alternatively could you open a new pr? i want to approve it :+1:
I did reset and force push but it seems the Pr somehow didn't get it. Will open a new one.
I did reset and force push but it seems the Pr somehow didn't get it. Will open a new one.
But the pr/repo owner could reopen it via github, or?
Not if the source branch has no commits:

@oneWaveAdrian Just in case you run into issues I still have a local checkout, so I could restore a branch as well.
@juliushaertl @ultrasites new PR as suggested: https://github.com/nextcloud/deck/pull/4156/ It is weird, the reason this happened was that I clicked on "Synch Fork" in my branch repo and then it deleted all my edits as it reset to master apparently... will do it through terminal again next time, thought I'd try this github feature to save time, oh well...