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

Tab change bug, DataView show/hide, bold titles #255

Merged
merged 6 commits into from
Jun 11, 2024
Merged

Conversation

CelineMP
Copy link
Contributor

@CelineMP CelineMP commented Jun 6, 2024

Fixed this bug: #212

There were two problems:

  • Clicking on "…" bubbled up to parent and always led to tab change
  • Tab Option Menu used the opened tab's id. So I made a new tab ID that is set when "…" is clicked

Signed-off-by: Celine Pöhl <[email protected]>
@CelineMP CelineMP added the frontend Issues related to the user interface of a web application label Jun 6, 2024
@CelineMP CelineMP requested review from Corgam and Lucas-Nan June 6, 2024 11:19
@Corgam
Copy link
Collaborator

Corgam commented Jun 6, 2024

I am so happy that you managed to fix it! I just have one comment that the Info popup option is not working correctly, and maybe if you can disable the black border when the tab is focussed that would be great ( when you click ... and click out of the popup somewhere on the screen)

image

EDIT: And I found another thing that if you click in the red region the tab does not change

Zrzut ekranu 2024-06-06 135859

@CelineMP
Copy link
Contributor Author

CelineMP commented Jun 6, 2024

Oh no true, the info is broken!
Yes the border looks bad. It shouldn't even be focusable, it should just change to the tab.

EDIT: Yes exactly. Wherever you click (besides the three dots), it should change.

@Corgam Corgam marked this pull request as draft June 9, 2024 13:10
@CelineMP
Copy link
Contributor Author

@Corgam I fixed the three mentioned things now:

  • Info PopUp bug
  • You can click everywhere on the Tab to change, not only the text
  • Removed focus outline

@CelineMP CelineMP marked this pull request as ready for review June 11, 2024 09:35
Signed-off-by: Celine Pöhl <[email protected]>
Signed-off-by: Celine Pöhl <[email protected]>
@CelineMP
Copy link
Contributor Author

Now this PR also includes:

The functionality is now there. But where exactly the hide/show button should be can be discussed next sprint. For now I put it next to the "new tab" button.

@CelineMP CelineMP changed the title Fixed tab change bug Tab change bug, DataView show/hide, bold titles Jun 11, 2024
Copy link
Contributor

@Lucas-Nan Lucas-Nan left a comment

Choose a reason for hiding this comment

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

bugs seem to fixed as far as i can see | button is temporary there but i think it should still be closer to the plus btn - so just not filled and maybe rounded corners
grafik
this looks too out of place for me

@Corgam
Copy link
Collaborator

Corgam commented Jun 11, 2024

bugs seem to fixed as far as i can see | button is temporary there but i think it should still be closer to the plus btn - so just not filled and maybe rounded corners grafik this looks too out of place for me

I agree, it would be nice to not create more work/tasks for the next sprints, just implement the final styling from the start :)
I would suggest using CaretDoubleRight icon (It would look better, but I think still the dataview needs a little cleanup with icons, there would be too many, maybe delete the pin icon next to the coordinates)

image

Signed-off-by: Celine Pöhl <[email protected]>
@Lucas-Nan
Copy link
Contributor

emils proposed variant doesnt support hiding the whole thing so nicely

@CelineMP
Copy link
Contributor Author

@Lucas-Nan

I implemented your feedback. But I personally prefer a different background color.

Bildschirmfoto 2024-06-11 um 17 46 31

@Corgam

The problem is: when you put the icon into the DataView, where is it when the DataView is hidden?

@Corgam
Copy link
Collaborator

Corgam commented Jun 11, 2024

@Lucas-Nan

I implemented your feedback. But I personally prefer a different background color.

Bildschirmfoto 2024-06-11 um 17 46 31 @Corgam

The problem is: when you put the icon into the DataView, where is it when the DataView is hidden?

I like it how it is right now after the changes, maybe just use the double arrows and I would be really happy haha

Signed-off-by: Celine Pöhl <[email protected]>
@CelineMP
Copy link
Contributor Author

I like it how it is right now after the changes, maybe just use the double arrows and I would be really happy haha

Done!
Bildschirmfoto 2024-06-11 um 17 52 09

@Lucas-Nan Lucas-Nan merged commit 787d543 into sprint-release Jun 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Issues related to the user interface of a web application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants